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 peter@xxxxxxxxxxxxxxxx 2007-03-13 00:36 EST ------- 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. :] 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.) == Unofficial Review of jokosher 0.9-1.20070308svn == ----- 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). 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. 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. :] 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. BAD: Your package does not have an appropriate %clean section. It should contain simply "rm -rf $RPM_BUILD_ROOT". See ----- GOOD :The spec file is named "%{name}.spec" in accordance with Naming Guidelines. GOOD: BuildRoot is "%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)", following the packaging guidelines. GOOD: Included documentation (%doc) is OK; and it seems to run well without these files installed. GOOD: Package includes an appropriate .desktop file since it is a graphical application. GOOD: Macros are used instead of hardcoded file names, and usage of these macros (including $RPM_BUILD_ROOT) is consistent throughout the spec file. GOOD: Locale files are handled correctly, using %find_lang appropriately. GOOD: Package is not relocatable. GOOD: Package includes appropriate code and content. GOOD: Package does not own any system files/directories or any files/directories that conflict with another package. GOOD: Package license (GPL) is OK; and a copy of it is included in the package as documentation (%doc COPYING). The License field in the spec file properly reflects this. GOOD: Spec file is legible and in American English. GOOD: No duplicates are listed in the %files section; and its %defattr line is good. GOOD: When installed, the application runs well with no apparent segfaults or major bugs. GOOD: Appropriate scriplets for MIME-type, Scrollkeeper, and .desktop files are used. GOOD: The %files section has a proper %defattr(-,root,root,-) line. ----- N/A: No non-ASCII characters are needed, so encoding is OK. N/A: This is a noarch package, so compiler flags are not needed. N/A: No static libraries or RPATH exclusions are needed. N/A: Package includes no configuration files or data, so %config markings are not needed. N/A: Package uses Python setuptools for building; so using `make %{?_smp_flags}` is not needed. N/A: Package is not a web application. N/A: Package is noarch; thus no ExcludeArch/ExclusiveArch tweaking is required. N/A: Package installs no shared libraries; thus %post/%postun scriplets of /sbin/ldconfig are not needed. N/A: No large (neither in size nor in quantity) documentation is included, thus no -doc subpackage is needed. N/A: No headers, no pkgconfig files, and no static or unsuffixed shared libraries are included. Thus, no -devel subpackage is needed. N/A: Package contains no libtool archives (*.la files) N/A: Package contains no %description or Summary translations. ----- 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. 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. -- 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