[Bug 618354] Review Request: cwallpaper - A front end for fbsetbg, Esetroot, feh, and other wallpaper changers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=618354

Ralph Lange <Ralph.Lange@xxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Ralph.Lange@xxxxxxxx

--- Comment #1 from Ralph Lange <Ralph.Lange@xxxxxxxx> 2010-09-08 17:47:20 EDT ---
Hi Germán,

I had a look at your package, which looks almost fine to me.

General question: You are packaging the 0.3.2 GTK version of cwallpaper, not
the newer versions (0.3.3 -> 0.4.2) that are based on fltk, which is also part
of Fedora. What is your reason for this?

Looking at the spec file:
 * Your summary could even be more concise and skip the "A" at the beginning.
 * Add "rm -rf %{buildroot}" to the beginning of the %install section 
   to get rid of rpmlint's no-cleaning-of-buildroot warning.

The build generates ~12 warnings, none of which seems critical.
Upstream might still be interested in fixing those.

Here's an informal review - I'm still working on my reviewing skills...

$ rpmlint cwallpaper.spec /var/lib/mock/fedora-rawhide-i386/result/cwallpaper*
cwallpaper.spec: W: no-cleaning-of-buildroot %install
cwallpaper.i686: W: spelling-error Summary(en_US) fbsetbg -> setback, subset,
besetting
cwallpaper.i686: W: spelling-error Summary(en_US) feh -> eh, fee, fen
cwallpaper.i686: W: spelling-error %description -l en_US fbsetbg -> setback,
subset, besetting
cwallpaper.i686: W: spelling-error %description -l en_US bsetbg -> subset,
setback, basset
cwallpaper.i686: W: spelling-error %description -l en_US esetroot -> beetroot,
esotropia, restroom
cwallpaper.i686: W: spelling-error %description -l en_US feh -> eh, fee, fen
cwallpaper.i686: W: spelling-error %description -l en_US bsetroot -> beetroot,
betroths, betroth
cwallpaper.i686: W: spelling-error %description -l en_US config -> con fig,
con-fig, configure
cwallpaper.i686: W: no-manual-page-for-binary cwallpaper
cwallpaper.src: W: spelling-error Summary(en_US) fbsetbg -> setback, subset,
besetting
cwallpaper.src: W: spelling-error Summary(en_US) feh -> eh, fee, fen
cwallpaper.src: W: spelling-error %description -l en_US fbsetbg -> setback,
subset, besetting
cwallpaper.src: W: spelling-error %description -l en_US bsetbg -> subset,
setback, basset
cwallpaper.src: W: spelling-error %description -l en_US esetroot -> beetroot,
esotropia, restroom
cwallpaper.src: W: spelling-error %description -l en_US feh -> eh, fee, fen
cwallpaper.src: W: spelling-error %description -l en_US bsetroot -> beetroot,
betroths, betroth
cwallpaper.src: W: spelling-error %description -l en_US config -> con fig,
con-fig, configure
cwallpaper.src: W: no-cleaning-of-buildroot %install
3 packages and 1 specfiles checked; 0 errors, 19 warnings.

A manual page is not available, the no-cleaning-of-buildroot warning can be
fixed, all others
seem to be false positives.
I don't see the unstripped-binary-or-object warning that you saw, and the
binary is stripped.

---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] 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.
[+] MUST: The package must be licensed with a Fedora approved license.
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: The 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.
    $ md5sum cwallpaper-0.3.2.tar.gz*
    85ca1399e8960097cbb6580dceb47163  cwallpaper-0.3.2.tar.gz
    85ca1399e8960097cbb6580dceb47163  cwallpaper-0.3.2.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: Packages storing shared library files must call ldconfig in %post and
%postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates.
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc 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: .so files must go in a -devel package.
[.] MUST: devel packages must require the base package.
[+] MUST: Packages must NOT contain any .la libtool archives.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[-] MUST: All filenames in rpm packages must be valid UTF-8.

[.] SHOULD: If the source package does not include license text(s) ...
[.] SHOULD: description and summary translations for supported Non-English
languages.
[+] 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.
[X] SHOULD: The reviewer should test that the package functions as described.
    Trying to run it in mock, I get:
        mock-chroot> cwallpaper 
        Attempting to use the config file: /builddir/.config/cwallpaper/config.
        Config file was not found, using internal defaults (Hope you have
fbsetbg :D ).
        Gtk-Message: Failed to load module "pk-gtk-module":
libpk-gtk-module.so: cannot open shared object file: No such file or directory
        Gtk-Message: Failed to load module "canberra-gtk-module":
libcanberra-gtk-module.so: cannot open shared object file: No such file or
directory

        (cwallpaper:3249): Pango-WARNING **: failed to choose a font, expect
ugly output. engine-type='PangoRenderFc', script='latin'
        Loading

        (cwallpaper:3249): Pango-WARNING **: failed to choose a font, expect
ugly output. engine-type='PangoRenderFc', script='common'

        (cwallpaper:3249): GLib-GObject-CRITICAL **: g_object_unref: assertion
`G_IS_OBJECT (object)' failed
        Quiting peaceablly
    And a window with no fonts displayed.
    I am not into GTK enough to decide if this is not supposed to work in mock,
or something that needs to be fixed.

[.] 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: pkgconfig(.pc) files should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin ...
[X] SHOULD: your package should contain man pages for binaries/scripts.
    Would be nice to have a manpage - but even the Debian packaged version
doesn't have one.

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]