Spec URL: http://www.boston.burdell.org/repoman/RPMS/source/repoman.spec SRPM URL: http://www.boston.burdell.org/repoman/RPMS/source/repoman-0.7-1.fc7.src.rpm Description: repoman is a graphical application for configuring yum(8) settings, enabling and disabling repositories, and configuring yum(8) plug-ins.
Hey David. Here's a review. It looks like you also need a sponsor? OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License(GPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: aba4be5ea7da8cb0f751e1400d509acf repoman-0.7.tar.gz aba4be5ea7da8cb0f751e1400d509acf repoman-0.7.tar.gz.1 OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Package has rm -rf RPM_BUILD_ROOT at top of %install See below - Package is a GUI app and has a .desktop file OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane. SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. OK - Should have dist tag OK - Should package latest version Issues: 1. The source url doesn't seem quite right http://www.boston.burdell.org/repoman/src/repoman-0.7.tar.gz works. (ie, it needs a /src/ in there) 2. rpmlint says: a) W: repoman no-dependency-on usermode Should "Requires: usermode" since you have a link to consolehelper. b) W: repoman incoherent-version-in-changelog 0.7 0.7-1.fc7 Should have the Release on the versions in the changelog... ie, 0.7-1 c) W: repoman conffile-without-noreplace-flag /etc/pam.d/repoman W: repoman conffile-without-noreplace-flag /etc/security/console.apps/repoman Are users ever likely to modify those files? Should they be noreplace? 2. You shouldn't need to require desktop-file-utils anymore, also you might use the standardized scriptlet for updating the mime-type key. See: http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef 3. You should use desktop-file-install to install the .desktop file: http://fedoraproject.org/wiki/Packaging/Guidelines#head-254ddf07aae20a23ced8cecc219d8f73926e9755 4. Is there a reason for the (8) after yum in the summary and description? I find it distracting, and many people won't know what it means. Finally two items that are by no means blockers, but I thought I would mention: - Perhaps you could talk with the yum-presto maintainer and see if it would be possible/easy to add support for deltarpm repos when they appear? - I see that this application doesn't have an icon. Perhaps you could ask for someone on the art group to whip one up? http://fedoraproject.org/wiki/Artwork/DesignService
(In reply to comment #1) > It looks like you also need a sponsor? Yes. > 1. The source url doesn't seem quite right > http://www.boston.burdell.org/repoman/src/repoman-0.7.tar.gz > works. (ie, it needs a /src/ in there) Fixed. > 2. rpmlint says: > > a) W: repoman no-dependency-on usermode > > Should "Requires: usermode" since you have a link to consolehelper. Fixed. > b) W: repoman incoherent-version-in-changelog 0.7 0.7-1.fc7 > > Should have the Release on the versions in the changelog... > ie, 0.7-1 Fixed. > c) > W: repoman conffile-without-noreplace-flag /etc/pam.d/repoman > W: repoman conffile-without-noreplace-flag /etc/security/console.apps/repoman > > Are users ever likely to modify those files? Should they be noreplace? Most likely users will never have to modify those files. But, they are config files and we wouldn't go to the trouble of making them config files if we didn't want to give the users the option of changing them. I've added the noreplace attribute. > 2. You shouldn't need to require desktop-file-utils anymore, also you > might use the standardized scriptlet for updating the mime-type key. See: > http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef Fixed. > 3. You should use desktop-file-install to install the .desktop file: > http://fedoraproject.org/wiki/Packaging/Guidelines#head-254ddf07aae20a23ced8cecc219d8f73926e9755 Fixed, I think. Not sure if I'm using this correctly. > 4. Is there a reason for the (8) after yum in the summary and description? > I find it distracting, and many people won't know what it means. Only to indicate it's a command with a man page. Removed the (8). > Finally two items that are by no means blockers, but I thought I would mention: > > - Perhaps you could talk with the yum-presto maintainer and see if it would > be possible/easy to add support for deltarpm repos when they appear? Definitely something to look in to. Added it to the TODO list. > - I see that this application doesn't have an icon. Perhaps you could ask for > someone on the art group to whip one up? > http://fedoraproject.org/wiki/Artwork/DesignService Also added to the TODO list. I have put all of these changes together in repoman-0.8. Here is the new srpm and spec file: http://www.boston.burdell.org/repoman/RPMS/source/repoman-0.8-1.fc7.src.rpm http://www.boston.burdell.org/repoman/RPMS/source/repoman.spec
Sorry for the delay, I was out on vacation... 1. ok. Looks good. 2.a. ok. Looks good. 2.b. ok. Looks good. 2.c. ok. Looks good. 2 (oops. I seem to have failed numbering here). ok. looks good. 3. That looks right to me on the desktop file install. 4. good. You seem to have removed the 'rm -rf $RPM_BUILD_ROOT' from the top of %install. That needs to be there... Humm... the 0.8 version doesn't seem to build here in mock: + desktop-file-install --vendor=fedora --delete-original --dir=/var/tmp/repoman-0.8-1.fc7-root-mockbuild/usr/share/applications/fedora /var/tmp/repoman-0.8-1.fc7-root-mockbuild/usr/share/applications/fedora/repoman.desktop Error on file "/var/tmp/repoman-0.8-1.fc7-root-mockbuild/usr/share/applications/fedora/repoman.desktop": Failed to open file '/var/tmp/repoman-0.8-1.fc7-root-mockbuild/usr/share/applications/fedora/repoman.desktop': No such file or directory error: Bad exit status from /var/tmp/rpm-tmp.16798 (%install)
Some more enhancement suggestions: - Would it make sense for repoman to allow you to enable/disable plugins? - Could you look at adding support for the priorities plugin? http://wiki.centos.org/PackageManagement/Yum/Priorities Just RFE's, nothing blocking this review.
David - I believe I've fixed the issues in comment #3. Want to check it out and do a new release?
New release that to fix up remaining review issues: http://www.boston.burdell.org/repoman/RPMS/source/repoman-0.9-1.fc7.src.rpm http://www.boston.burdell.org/repoman/RPMS/source/repoman.spec The RFEs posted here are all great ideas. I'd rather not post there here since this bug is just for the review though.
The package from comment #6 seems to have all blockers I can see fixed. So, this package is APPROVED. Don't forget to close this review request once it's been imported and built. It looks like you added the RFE's from here to the TODO file, which is a fine place for them. ;)
New Package CVS Request ======================= Package Name: repoman Short Description: Tool for configuring yum settings and repositories Owners: dcantrell,clumens Branches: InitialCC: