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=562585 Orcan 'oget' Ogetbil <oget.fedora@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |oget.fedora@xxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |oget.fedora@xxxxxxxxx Flag| |fedora-review? --- Comment #10 from Orcan 'oget' Ogetbil <oget.fedora@xxxxxxxxx> 2010-07-08 22:02:31 EDT --- I am taking the review. The package is mostly in good shape. There are minor things to fix. I made the review a little verbose since this is your first package. Since you need to be sponsored, we expect a little more from you. To show that you understand and work comfortably with the Fedora guidelines, you will need to do either some informal reviews on packages that are awaiting a review (this is the preferred method) http://fedoraproject.org/PackageReviewStatus/ or submit some other packages for review, that are preferably different type of packages. For more detail, see http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group Here is the review: - rpmlint says: ccd2iso.x86_64: W: no-manual-page-for-binary ccd2iso It would be nice to have a man page. But this is not a blocker. ! Usually the EOL encoding erros are fixed via "sed" or "dos2unix". See http://fedoraproject.org/wiki/Packaging/Guidelines#Rpmlint_Errors A typical way of doing it is: sed 's/\r//' TODO > TODO.tmp touch -r TODO TODO.tmp mv -f TODO.tmp TODO or dos2unix -k TODO So that you preserve the timestamp of the file. Note that for dos2unix, you need to add a BuildRequires. I would say the patch is okay since the file is really small, but I would have preferred one of the above two. ! We don't package the INSTALL files, since they usually tell us how to compile the source. This is not relevant for an RPM user. * The license of the package is determined as follows: - You look at the license file (if it is missing you notify upstream). This one says GPLv2. - We look at the source code. The source code is in directory src/ in this case. - The header of the source code files carry the phrase: * This program is free software; you can redistribute it and/or modify * * it under the terms of the GNU General Public License as published by * * the Free Software Foundation; either version 2 of the License, or * * (at your option) any later version. * Since it says "or any later version", the license of the package must be GPLv2+. (Sometimes the source code does not carry the "or any later version" phrase. In that case we set the license tag to GPLv2. Sometimes the source code does not even specify the GPL version. In this case, even if the COPYING file says GPLv2 or GPLv3, we set the license tag to GPL+, and we notify upstream to add the GPL versions to the source file headers) For more information, see http://fedoraproject.org/wiki/Licensing#Good_Licenses scroll down to "GNU General Public License (no version)" ! You can use the %{name} macro in %files to keep macro usage consistent. * The compiler gives these warnings: ccd2iso.c:61: warning: implicit declaration of function 'strcmp' This can be avoided by adding a #include <string.h> ccd2iso.c:97: warning: format '%d' expects type 'int', but argument 2 has type 'long unsigned int' Replacing the %d with %zd will fix this warning. You can write a patch to fix these and send it upstream. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review