Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: jokosher https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199029 ------- Additional Comments From snecklifter@xxxxxxxxx 2007-03-16 19:58 EST ------- Hello Peter, (In reply to comment #69) > I can't sponsor you, but here's a brief unofficial review of your spec/SRPM to > help expedite getting this into Fedora because it's such an awesome little app. :] I'm glad you agree - its been a long time coming :P > Unfortunately, Mock is also quite broken at the moment, so I cannot post > comments about its build-time structure or quality. (However, it seems to run > okay through a standard rpmbuild invocation; and rpmlint is silent on that built > RPM.) Mock is working for me and builds fine with the following changes. rpmlint is quiet. > BAD: The package does not follow the Naming Guidelines: The Release tag should > be "0.%{X}.%{alphatag}" or equivalent as noted in the "Pre-Release packages" > section of the naming guidelines (Packaging/NamingGuidelines on the wiki). This should now be fixed as I have started bumping the %{X} section with each new build. > BAD: You have duplicate BuildRequires listed: python-devel and pycairo-devel are > both dependencies of pygtk2-devel. Thus, simply listing pygtk2-devel as a > BuildRequires should pull in the other two automagically; and you should remove > those two from the BuildRequires list. Fixed. > BAD: %{_datadir}/omf/%{name}/, %{_datadir}/gnome/help/%{name}/, and > %{_datadir}/gnome/help/%{name}/C/ should all be owned by the package. Otherwise > they are orphaned when the package is removed. We don't like clutter in our > filesystems. :] Fixed. > BAD: The package should invoke desktop-file-install to install the .desktop file > in your %install section. See the "desktop-file-install usage" section on the > wiki's Packaging/Guidelines page for more details. Fixed. > BAD: Your package does not have an appropriate %clean section. It should contain > simply "rm -rf $RPM_BUILD_ROOT". See Fixed. > MINOR(?): The source tarball does not match that of upstream (in this case, the > upstream tarball is generated from the svn export as you explain in your spec > comments): > $ md5sum SOURCES/jokosher-0.9* > 24ae1fa6adad7893a58fcd32aaec1ff3 jokosher-0.9-srpm.tar.gz > 26837769e637a9225fb26afe243488db jokosher-0.9-upstream.tar.gz > However, if I grab another SVN snapshot and tar it up, the md5sum changes yet > again, so this seems to be a simple false positive; and is probably safe to ignore. Okay, let me know if this becomes a problem. Note I am doing an svn export as opposed to a svn checkout to lose all the usual svn cruft. > MINOR: It's just preference, but instead of listing the directory via %dir and > globbing all of the files inside of it, it's probably simpler to simply list the > directory itself as a parent. RPM will automagically make it grab all the files > and directories in that recursively. E.g., instead of the current: > %dir %{python_sitelib}/Jokosher > %{python_sitelib}/Jokosher/*.py > %{python_sitelib}/Jokosher/*.pyo > %{python_sitelib}/Jokosher/*.pyc > You could use something like: > %{python_sitelib}/Jokosher/ > Your %exclude line should still hold, so that's ok. Fair point. Hopefully it now looks cleaner. http://www.iammetal.co.uk/jokosher/ Cheers Chris -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review