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=498218 --- Comment #8 from Edwin ten Brink <fedora@xxxxxxxxxxxxxxxxxxx> 2009-11-08 10:23:49 EDT --- Orcan, thanks for your review. My responses marked with + have been incorporated, -, not, and ? are pending your response (not yet incorporated). (In reply to comment #6) > * The license is MIT > https://fedoraproject.org/wiki/Licensing/MIT > old style. One of the oldest software licenses out there. + Thanks, missed that one. > ! Guidelines say: "If (and only if) the source package includes the text of the > license(s) in its own file, then that file, containing the text of the > license(s) for the package must be included in %doc." in > http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text"; > But this package does not even have a tarball. Is there any other examples of > Fedora packages that just cut the license text from the soure file? If not, I > would say it is best to obey the guideline. ? I'm unaware of any such practices (or cases where it might be applicable for that matter). I preferred to have the README out of the script in a %doc file, and figured when I did so, I could just as well generate a COPYING. It seems that I either should do it this way, or drop all generated %doc files entirely. Which option do you recommend? > ? Shall we rename the executable to just picturetile? That is the upstream > project name. - I agree that an extension of .pl is not necessary, but fspot looks for an executable picturetile.pl in $PATH, so unless we want a symlink from picturetile.pl to picturetile, I suggest to drop this comment. > ! It would be better to install the executable with "install -pm 755 ..." and > not use %attr(0755,root,root) to comply with general Fedora conventions.. + Agreed. > ? I am a bit reluctant about the provides: > Provides: picturetile.pl = 20050314 > Do we really need this? ? No, not really. I just added it in case someone decides to make a Requires in fspot. I can drop this without any adverse effects, should I? > ? I would go with the suggestion in comment #2 for version and release numbers. > Even if upstream decides to make a 0.1 release we'll be in trouble. + Ok. Although the version number is now fool proof, I doubt that it will be necessary. > * The first 5 lines of the %build should probably go into %prep. Also could you > use the "%setup -qcT" macro in %prep so that the package gets built in its own > directory? + Agreed. > ! ah. also it would be nice to have the source file downloaded with > wget -N http://... > so we have the correct timestamp. + Agreed. Though I unfortunately need some fussing around with timestamping since the file has to be converted to UTF-8. ? I'm not quite happy with the hardcoded URL in two places though. Isn't there a way to refer to Source0 directly (without rpm rewriting it to the local SOURCES directory)? Changes so far have been included in the following files: Spec URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile.spec SRPM URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile-0.1.20050314-3.fc11.src.rpm -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review