Re: RFS: flvmeta (updated package)

[Neutron Soutmun <neo.neutron@gmail.com>]

On Wed, Nov 23, 2011 at 9:43 AM, Paul Wise <pabs@debian.org> wrote:
> On Tue, Nov 22, 2011 at 4:44 PM, Neutron Soutmun wrote:
>
>> flvmeta    - Metadata injector for FLV video files
>
> The package is already uploaded, but here is a review anyway:
>
> http://packages.qa.debian.org/f/flvmeta.html
>
> You might want to consider using wrap-and-sort.

Brilliant, I just know that Debian already give me a tool like this :)

> I think your watch file should use can=1 instead of can=3.

Changed, will be included in the next upload.

> In the watch file there needs to be a uversionmangle/dversionmangle
> set so that versions can be properly compared.

Learning from uscan manpage, and should be updated in the next upload.

> You can probably drop the roff documentation from the manual page,
> lines starting with this: .\"
> Please forward the manual page upstream if you have not already.

Considered to not change by now as upstream accepts the manpage patch already.
http://code.google.com/p/flvmeta/source/detail?r=f49070b489892d7f39e84b73cc85293987977623

Maybe, in the next upstream version should includes the manpage and
the Debian manpage should be dropped.

> In your patches it is customary to set the Forwarded header to the URL
> where the patch can be found upstream or the email address it was
> forwarded to.

Updated for next upload.

> There is one GCC warning:
>
> json.c: In function 'json_unescape':
> json.c:1337:7: warning: format '%llX' expects argument of type 'long
> long unsigned int', but argument 3 has type 'long unsigned int'
> [-Wformat]

I was sent the patch and already accepted before.
http://code.google.com/p/flvmeta/issues/detail?id=38&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary%20Branch

- fprintf (stderr, "JSON: unsupported unicode value: 0x%llX\n",
(uint64_t)unicode);
+ fprintf (stderr, "JSON: unsupported unicode value: 0x%llX\n",
(unsigned long long)unicode);

with this patch, compiled without warning. Wondering, I think that
"uint64_t" = "unsigned long long int".

> Please modify the debtags for this package:
>
> http://debtags.alioth.debian.org/edit.html?pkg=flvmeta
> Please consider uploading a screenshot of flvmeta in use in a terminal:
>
> http://screenshots.debian.net/package/flvmeta

I will.

Thank you very much.

Best regards,
Neutron Soutmun