[Bug 751820] Review Request: pius - A tool for signing and emailing all UIDs on a set of PGP keys.

Wed Nov 9 14:41:11 UTC 2011

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=751820

--- Comment #4 from Praveen Kumar <kumarpraveen.nitdgp at gmail.com> 2011-11-09 09:41:10 EST ---
(In reply to comment #3)
> Sorry about that, the issue is that i tried in a RHEL5 machine.
> 
> This is an informal review, for the purpose of my sponshorship, tracked here:
> https://bugzilla.redhat.com/show_bug.cgi?id=749132
> 
> General comments:
> 
> 1) I would put the actual name in the url, not using the %{name} tag (easier to
> copy paste)
But It's always good to use macros and tags.
> 
> 2) Package should probably be noarch, see below for more
done
> 
> 3) Why do you package the spec file in the binary rpm?
That is a sample spec file which is with source so I have to put.
> 
> 4) It would be nice to get man pages.
I already mailed to upstream but did not get any reply yet
> 
> 5) And a wish, consider adding any tests to %check if you have them (not a must
> of course).
> 
>  +:ok, =:needs attention, -:needs fixing
> 
> MUST Items:
> [-] MUST: rpmlint must be run on every package.
> 
> rpmlint is not silent:
> W: summary-ended-with-dot C A tool for signing and email all UIDs on a set of
> PGP keys.
> - should be an obvious fix
> 
> W: spelling-error %description -l en_US keysigning -> key signing, key-signing,
> designing
> - key-signing?
> 
> E: no-binary
> - stuff in %{_bindir} are python and perl scripts, should it be noarch?
> 
> W: no-manual-page-for-binary pius-keyring-mgr
> W: no-manual-page-for-binary pius-party-worksheet
> W: no-manual-page-for-binary pius
> - please consider adding man pages for all commands
> http://fedoraproject.org/wiki/Packaging:Guidelines#Man_pages
> 
> E: empty-debuginfo-package
> - debuginfo rpm is empty, same as above, package should probably be noarch
> 
> # rpmlint -I empty-debuginfo-package
> empty-debuginfo-package:
> This debuginfo package contains no files.  This is often a sign of binaries
> being unexpectedly stripped too early during the build, rpmbuild not being
> able to strip the binaries, the package actually being a noarch one but
> erratically packaged as arch dependent, or something else.  Verify what the
> case is, and if there's no way to produce useful debuginfo out of it, disable
> creation of the debuginfo package.
> 
> 3 packages and 0 specfiles checked; 2 errors, 7 warnings.
> 
> [+] MUST: The package must be named according to the Package Naming Guidelines.
> [+] MUST: The spec file name must match the base package %{name}
> [+] MUST: The package must meet the Packaging Guidelines. [FIXME?: covers this
> list and more]
> [+] MUST: The package must be licensed with a Fedora approved license and meet
> the Licensing Guidelines.
> [+] MUST: The License field in the package spec file must match the actual
> license.
> [+] MUST: If (and only if) the source package includes the text of the
> license(s) in its own file, then that file, containing the text of the
> license(s) for the package must be included in %doc.
> [+] MUST: The spec file must be written in American English.
> [+] MUST: The spec file for the package MUST be legible.
> [+] MUST: The sources used to build the package must match the upstream source,
> as provided in the spec URL.
> # md5sum pius-2.0.9.tar.bz2*
> 6ea4b2eabf50f2d40aabe21a22c46bc4  pius-2.0.9.tar.bz2
> 6ea4b2eabf50f2d40aabe21a22c46bc4  pius-2.0.9.tar.bz2_srcrpm
> 
> [=] MUST: The package must successfully compile and build into binary rpms on
> at least one supported architecture.
> - it builds fine, but see above regarding noarch
> 
> [+] MUST: If the package does not successfully compile, build or work on an
> architecture, then those architectures should be listed in the spec in
> ExcludeArch.
> [-] MUST: All build dependencies must be listed in BuildRequires
> - i don't think you need python-devel in the BuildRequires (just python in
> Requires)
we need python-devel as BuildRequires
http://fedoraproject.org/wiki/Packaging:Python#BuildRequires
> 
> [+] MUST: The spec file MUST handle locales properly. This is done by using the
> %find_lang macro.
> [+] MUST: Every binary RPM package which stores shared library files (not just
> symlinks) in any of the dynamic linker's default paths, must call ldconfig in
> %post and %postun.
> [+] MUST: If the package is designed to be relocatable, the packager must state
> this fact in the request for review
> [+] MUST: A package must own all directories that it creates. If it does not
> create a directory that it uses, then it should require a package which does
> create that directory.
> [+] MUST: A package must not contain any duplicate files in the %files listing.
> [+] MUST: Permissions on files must be set properly. Executables should be set
> with executable permissions, for example
> - and %defattr not required anymore
> [+] MUST: Each package must have a %clean section, which contains rm -rf
> %{buildroot} (or $RPM_BUILD_ROOT).
> - not required for >= F13
> [+] MUST: Each package must consistently use macros, as described in the macros
> section of Packaging Guidelines.
> [+] MUST: The package must contain code, or permissible content. This is
> described in detail in the code vs. content section of Packaging Guidelines.
> [+] MUST: Large documentation files should go in a doc subpackage.
> [+] MUST: If a package includes something as %doc, it must not affect the
> runtime of the application.
> [+] MUST: Header files must be in a -devel package.
> [+] MUST: Static libraries must be in a -static package.
> [+] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
> (for directory ownership and usability).
> [+] MUST: If a package contains library files with a suffix (e.g.
> libfoo.so.1.1), then library files that end in .so (without suffix) must go in
> a -devel package.
> [+] MUST: In the vast majority of cases, devel packages must require the base
> package using a fully versioned dependency: Requires: %{name} =
> %{version}-%{release} 
> [+] MUST: Packages must NOT contain any .la libtool archives, these should be
> removed in the spec.
> [+] MUST: Packages containing GUI applications must include a %{name}.desktop
> file, and that file must be properly installed with desktop-file-install in the
> %install section.
> [+] MUST: Packages must not own files or directories already owned by other
> packages.
> [+] MUST: At the beginning of %install, each package MUST run rm -rf
> %{buildroot} (or $RPM_BUILD_ROOT).
> - not required anymore
> [+] MUST: All filenames in rpm packages must be valid UTF-8.
> 
> SHOULD Items:
> [+] SHOULD: If the source package does not include license text(s) as a
> separate file from upstream, the packager SHOULD query upstream to include it.
> [+] SHOULD: The description and summary sections in the package spec file
> should contain translations for supported Non-English languages, if available.
> [+] SHOULD: The reviewer should test that the package builds in mock.
> [=] SHOULD: The package should compile and build into binary rpms on all
> supported architectures.
> - noarch? see above
> 
> [+] SHOULD: The reviewer should test that the package functions as described.
> - tried simple execution and it seems to work
> [+] SHOULD: If scriptlets are used, those scriptlets must be sane.
> [+] SHOULD: Usually, subpackages other than devel should require the base
> package using a fully versioned dependency.
> [+] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and
> this is usually for development purposes, so should be placed in a -devel pkg.
> A reasonable exception is that the main pkg itself is a devel tool not
> installed in a user runtime, e.g. gcc or gdb.
> [+] SHOULD: If the 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.
> [-] SHOULD: Packages should try to preserve timestamps of original installed
> files.
> http://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps
Done

-- 
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.