On Wed, Nov 23, 2011 at 9:43 AM, Paul Wise <[email protected]> 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 -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
