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: snake - a client/server python framework used to support anaconda installations https://bugzilla.redhat.com/show_bug.cgi?id=391091 ------- Additional Comments From jwilson@xxxxxxxxxx 2007-11-26 13:24 EST ------- Latest pass through the spec: 1) the lines between %prep and %setup probably shouldn't be there, %setup is a macro used in the %prep section, not a new section. :) 2) I still see 6 /var and 2 /etc used instead of %{_localstatedir} and %{_sysconfdir}, respectively. Outside of that, yeah, rpmlint output is completely clean now, spec looks nice and tidy, resulting packages look sane, etc... Okay, on to more in-depth formal review... Fedora Package Review: snake ---------------------------- MUST Items: * rpmlint is silent * meets Package Naming Guidelines * spec file name matches %{name}, in the format %{name}.spec * package meets the Packaging Guidelines * open-source compatible license and meets fedora legal reqs * License field in spec matches actual license **** NOTE: **** Interesting tidbit here... You have both a COPYING and a LICENSE file, which are *almost*, but *not quite* the same file... You really only need one or the other, and it looks like the main difference between them are the address listed for the FSF and Lesser vs. Library. * source includes text of license(s) in its own file, file in %doc * spec file legible and in American English * sources used match the upstream source, as provided in spec URL. Verify with md5sum (if no upstream URL, source creation method must be documented and can be verified using diff). **** FIXME **** Oops. I missed this on the initial passes through the spec... Granted, its a git snap, but in that case, we need a bit of documentation on how the snap was produced. And once a 'stable' release is out, there ought to be tarballs available that you point to. Maybe check out how Jesse does things for Pungi. * produces binary rpms on x86_64/F8 just fine * No ExcludeArch used * BuildRequires are sane (only needs python-devel) * no locales * no shared libs * not relocatable * package owns all directories it creates: **** FIXME **** Just noticed that it doesn't look like the resulting binaries own /var/lib/snake. Just need to add a '%dir %{_localstatedir}/lib/snake' above the lines in '%files server' that own the sub-directories of that directory. * no duplicates in %files * permissions on %files sane * %clean includes rm -rf $RPM_BUILD_ROOT * macros used consistently **** FIXME **** Just need to get the /var and /etc replacements in there. * package contains code * no need for a -doc subpackage * files in %doc aren't required for package to work * no header files * no static libs * no pkgconfig * no libs * no -devel * no libtool archives * not a GUI app * doesn't own files or folders other package own * %install starts with rm -rf $RPM_BUILD_ROOT * filenames in packages are valid UTF-8 SHOULD Items (not absolutely mandatory, but highly encouraged) * license text(s) included * description and summary sections in spec should contain translations for supported Non-English languages, if available: not available * package should build in mock: builds fine in x86_64 devel mock chroot. * package should build on all supported architectures: noarch package, no reason to think it won't build everywhere. * package should function as expected: untested by me personally, but known to be functional. * any scriptlets must be sane: server start/stop/add scriptlets are minimal and perfectly sane. * subpackages other than -devel require the base package using a fully versioned dependency: snake-server does so. * pkgconfig files go in -devel pkg, unless package is a devel tool itself: no pkgconfig stuff to worry about. * If package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself: n/a. So only a few little things to fix up before I think its ready to approve. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review