Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=914996 --- Comment #7 from Stephen Gordon <sgordon@xxxxxxxxxx> --- (In reply to comment #6) > Hi Stephenm, here is my first comment iteration :) > > ======= > > 1. bad spec naming? > > > Upstream have already updated to resolve this so I've updated the SPEC and > > SRPM to pick this up. I also realized that there is .pod file provided which > > can be used to generate the man page so that is now also included. > > > > Spec URL: http://fedorapeople.org/~sgordon/gitstats-0.2.spec > > SRPM URL: http://fedorapeople.org/~sgordon/gitstats-0.2-20130224git0843039.fc18.src.rpm > > Why do you now call the spec file gitstats-0.2.spec and not gitstats.spec? > Your > first specfile was called gitstats.spec.. I've actually had reviewers request that I do that in the past so they can see the differences between the submitted files (rather than overwriting each time). > 2. unnecessary %attr() macros > > > $ cat *.spec > > [...] > > %files > > %attr(755, root, root) %{_bindir}/%{name} > ^^^^^^^^^^^^^^^^^^^^^^ none of these should be needed Removed. > 3. Missing release number > > > Release: %{checkout}%{?dist} > > I think that even the first release number should be specified. Partly > because > I (as a reviewer) can see, where it will be finally placed and because of > this > also: > > $ rpmdev-vercmp gitstats-0.2-20130224git0843039 > gitstats-0.2-1.20130224git0843039 > gitstats-0.2-20130224git0843039 > gitstats-0.2-1.20130224git0843039 I went back and reviewed the naming/versioning guidelines and have ended up with names of the form gitstats-0-0.3.20130224git0843039. I reset the version to 0 as this is effectively "pre-release software" (no formal releases) and use the revision number to maintain the ordering as shown in the example: http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages > 4. (nit: I would create one-newline separator between changelog entries) Done > 5. Gzip inside BuildRequires is redundant .. it must be installed on minimum > build system. Removed > Pavel Thanks Pavel, updated files are here: Spec URL: http://fedorapeople.org/~sgordon/gitstats.spec SRPM URL: http://fedorapeople.org/~sgordon/gitstats-0-0.3.20130224git0843039.fc18.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=rOwuBPrChy&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review