[Bug 529496] Review Request: libmtag - An advanced C music tagging library with a simple API

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=529496





--- Comment #2 from Michael Schwendt <mschwendt at gmail.com>  2009-10-25 16:36:53 EDT ---
> Summary:        An advanced C music tagging library with a simple API

It's widely accepted practise to omit leading articles, such as "An" and "A" to
shorten summaries even further:

  Summary: Advanced C music tagging library with a simple API

That also looks better when displayed during installation.


> Source0:        http://libmtag.googlecode.com/files/%{name}-%{version}.tar.gz

0.3.2 is not available there. 404 Not Found.


* Spelling errors:  sed -i 's!excercise!exercise!g' libmtag.spec


> License:        LGPLv2

You should include the LGPL text in your tarball and confirm the licensing in
the source files:
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text


> Requires: taglib

Found in the pkgconfig file. And this dependency is not true. It would only be
true if you had a build-time dependency on taglib headers or static library.


> mtag.cpp

Is there any reason why you include C headers instead of their Standard C++
Library counterparts, such as <cstdio> and <cstring>?


* Please also take a look at the compiler warnings related to mtag.cpp and
mtag.c. Currently:

mtag.cpp:38: warning: suggest parentheses around assignment used as truth value
mtag.cpp:79: warning: suggest parentheses around assignment used as truth value
mtag.cpp:95: warning: suggest parentheses around assignment used as truth value
mtag.cpp:113: warning: suggest parentheses around assignment used as truth
value
mtag.cpp:126: warning: no return statement in function returning non-void
mtag.cpp:136: warning: suggest parentheses around assignment used as truth
value
mtag.cpp:143: warning: suggest parentheses around assignment used as truth
value
mtag.cpp:150: warning: suggest parentheses around assignment used as truth
value
mtag.cpp:310: warning: suggest parentheses around assignment used as truth
value
mtag.cpp:331: warning: suggest parentheses around assignment used as truth
value
mtag.c:179: warning: implicit declaration of function 'basename'
mtag.c:179: warning: format '%s' expects type 'char *', but argument 2 has type
'int'


> libmtag-tools

$ mtag
Error: Bad arguments: No filename specified

$ mtag --help
mtag: unrecognized option '--help'
Error: Bad arguments: No filename specified

$ mtag test1.mp3 
Error: Bad arguments: No filename specified

$ mtag test1.ogg
Error: Bad arguments: No filename specified


Without the README (which you don't include in the package), the mtag tool is
not very helpful about telling how to use it.

Run "rpmlint" on all the packages (src.rpm and built rpms).
https://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint

It would have warned about lack of documentation files. ;)

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.