[Bug 1388945] Review Request: gnome-shell-extension-media-player-indicator - Control MPRIS2 capable media players : Rhythmbox, Banshee, Clementine and more

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

 



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

Andrew Toskin <andrew@xxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrew@xxxxxxx



--- Comment #8 from Andrew Toskin <andrew@xxxxxxx> ---
I can do a formal review. To be succinct, I won't include the MUSTs and SHOULDs
that pass.

[!]: License field in the package spec file matches the actual license.

        Your spec file claims the license is GPLv3+, but the package's included
        COPYING file is a copy of the GPLv2. Version 3 might be more accurate,
        because the license comment at the top of widget.js says version 3,
        which should therefore apply to the whole project. Still, COPYING does
        not match your spec. Upstream should resolve this issue.

[!]: Package functions as described.

        The extension works with Rhythmbox, but does nothing if I'm playing
        music with Banshee instead.

        I'm testing this extension based on the latest SRPM you've linked,
        installed on Fedora 25 Workstation x86_64, with GNOME Shell 3.22.3,
        and Banshee 2.6.2. I do have several other extensions installed, so I
        suppose it's possible there's some conflict...

        * gnome-shell-extension-alternate-tab.noarch
        * gnome-shell-extension-apps-menu.noarch
        * gnome-shell-extension-auto-move-windows.noarch
        * gnome-shell-extension-background-logo.noarch
        * gnome-shell-extension-common.noarch
        * gnome-shell-extension-drive-menu.noarch
        * gnome-shell-extension-gpaste.noarch
        * gnome-shell-extension-launch-new-instance.noarch
        * gnome-shell-extension-media-player-indicator.noarch
        * gnome-shell-extension-places-menu.noarch
        * gnome-shell-extension-pomodoro.x86_64
        * gnome-shell-extension-sustmi-windowoverlay-icons.noarch
        * gnome-shell-extension-user-theme.noarch
        * gnome-shell-extension-window-list.noarch
        * gnome-shell-extension-windowsNavigator.noarch

        Curiously, Media Play Indicator does not appear in GNOME Tweak Tool
        either. I had to enable it from the command line with
        gnome-shell-extension-tool.

[x]: Latest version is packaged.

        Hard to say, since the upstream GitHub repo does not tag its releases.
        The commit that you use as the snapshot is recent, so I'm assuming it's
        okay. However, the package versioning guide say that when upstream has
        never tagged a release, the version tag should simply be set to 0 (not
        0.1). Or is the idea here that you're packaging a *prerelease* of the
        eventual upstream 0.1 release?
        <https://fedoraproject.org/wiki/Packaging:Versioning>

[!]: Packages should try to preserve timestamps of original installed files.

        This is not mandatory, but I think it's meant to help with verifying
        the source files. If no one has asked the upstream about this yet, you
        might ask them to consider editing their Makefile to use `cp -p` or
        `install -p` or equivalent.

rpmlint complains about "explicit-lib-dependency glib2". I guess you don't need
to include that in your dependencies.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




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