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:
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.
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
1. The source url doesn't seem quite right
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...
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:
3. You should use desktop-file-install to install the .desktop file:
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?
(In reply to comment #1)
> It looks like you also need a sponsor?
> 1. The source url doesn't seem quite right
> 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
> 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
> 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:
> 3. You should use desktop-file-install to install the .desktop file:
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?
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:
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.
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
Error on file
Failed to open file
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?
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:
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