Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337 --- Comment #8 from Hans de Goede <hdegoede@xxxxxxxxxx> --- Hi, (In reply to comment #7) > (In reply to comment #6) > > Full review done: > > > > Good: > > - ... > > - package meets naming guidelines (but not versioning, see below) > > I'm looking below and I don't see more about the versioning, can you clarify > this for me? My bad, usually we only allow numeric chars in Version:, so I was planning to point this out and point you to: http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-Numeric_Version_in_Release But while reading that myself I noticed that there is one exception to the only numeric chars in the Version field, and this package matches that exception, so its use I fine, Then I removed the comment on this, but not the referral to the comment you just quoted. > > -The generated manpage is rubish, it contains the same bit twice, and then a > > reference to the non-existent texinfo documentation. Luckily Debian has > > already packaged nmon and provides a manpage for us :) See: > > ftp://ftp.nluug.nl/pub/os/Linux/distr/debian/pool/main/n/nmon/ > > nmon_13g+debian-1.debian.tar.gz > > The Debian manpage has also been committed on sourceforge under patches, so > I downloaded that one. However, I'm not able to follow the sourceforge > guidelines for this link. > > There's a second manpage, which covers more of the documentation, and I > would like to use that one, but I has some unwanted references to a > screenshot list in its intro. I'm considering rewriting it a bit and then > use it, what are your thoughts on that? > > link: > http://sourceforge.net/tracker/ > ?func=detail&aid=3295760&group_id=271307&atid=1153693 That is fine. But please change the LICENSE section from "under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version." To: "under the terms of the GNU General Public License as published by the Free Software Foundation; version 3" IOW drop the "either" and the ", or (at your option) any later version.", as the only license statement we've is the one from Documentation.txt which does not contain the or later version language. You should then also submit the updated manpage upstream (not sure how useful that is in this case, but as a rule we always try to send all changes upstream in Fedora). > > > -Drop the now no longer needed "BuildRequires: help2man" > > Now using install -D -p -m 0655 for the nmon.1 manpage. That should be -m 0644 which I see you've gotten right in the specfile :) > > ... you > > *MUST* mail upstream asking them to do better for their next release. While > > mailing upstream, please attach the Debian manpage and ask them to add that > > to the tarbal too. > > I have mailed Nigel using sourceforges mailing system, as I'm unable to find > any other contact information, and asked for inclusion of license file and > header, manpage, and tarball releases. Great, thanks. I know this rule may seem a bit stupid with upstreams which appear dead. But it is a good rule in general, and in my experience with dead upstreams sometimes they surprise you by not being dead after all. > The projects mailing list has a total > of 2 messages, from the same person as he never received an answer, I'm > guessing nobody reads it. > > I could make feature requests for the above, but I'm not a fan of that, as > it has nothing to do with the functionality of the program. No need to file an RFE, if a direct mail won't do the trick usually nothing will. > New files > Spec URL: http://nmon.zom.dk/nmon.spec > SRPM URL: http://nmon.zom.dk/nmon-14g-3.fc18.src.rpm Looks good: approved! So as promised I've added you to the packager group and sponsored you, so you can now move forward with: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner 2 minor nitpicks to further improve the package: 1) In the ChangeLog you write: - Remove manpage from doc This may lead to users thinking the manpage was removed. If I were you I would've written this instead: - No longer mark manpage as %%doc Note the %%, whenever you use % in the changelog (or in comments) you need to write %% 2) In %prep you do: cp %{SOURCE2} . and then in %install: install -D -p -m 0644 %{name}.1 %{buildroot}%{_mandir}/man1/%{name}.1 Instead you can just do the following in %install, without anything in %prep: install -D -p -m 0644 %{SOURCE2} %{buildroot}%{_mandir}/man1/%{name}.1 Regards, Hans -- 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=EdGyWKOfz1&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review