Bug 1201925 - Review Request: gnome-shell-extension-touchpad-indicator - Minimalistic Touchpad management extension for the Gnome Shell
Summary: Review Request: gnome-shell-extension-touchpad-indicator - Minimalistic Touch...
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-03-13 20:33 UTC by Ville Skyttä
Modified: 2015-11-03 19:05 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-11-03 19:05:50 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ville Skyttä 2015-03-13 20:33:35 UTC
https://scop.fedorapeople.org/packages/gnome-shell-extension-touchpad-indicator.spec
https://scop.fedorapeople.org/packages/gnome-shell-extension-touchpad-indicator-0-1.20150221gitf2f30f4.fc23.src.rpm

Touchpad Indicator is a minimalistic Touchpad management extension for
the Gnome Shell. allows you to switch your Touchpad On or Off, if a
Trackpoint, a Fingertouch, a Touchscreen or a Pen exists it is also
possible to switch it On or Off through the extension. There's further
the possibility to automatically switch On or Off the touchpad and/or
trackpoint if a mouse is detected at startup or if a mouse is plugged
or the automatic switch off is disabled by default.

Fedora Account System Username: scop

Note to reviewer: I'm not sure if a dependency on xorg-x11-drv-synaptics should be added, i.e. if this package only works with it or if there are some other touchpad implementations that this stuff works with too. If you know, please advice and I can add the dependency. I do know that this works for my synaptics based touchpad even if I remove the /usr/bin/synclient executable so at least that executable is not a dependency.

Comment 1 Alexander Ploumistos 2015-04-10 21:35:14 UTC
Hi,

This is an informal review and it is also my first, so in all likelihood, I have missed something.

Your spec file looks good. You don't have any of the obsolete commands and sections.

When I ran it through rpmlint there where only a few warnings concerning the spelling of certain words - most of them highly debatable in my opinion:

gnome-shell-extension-touchpad-indicator.src: W: spelling-error Summary(en_US) Minimalistic -> Minimalist, Minimalism, Animistic
gnome-shell-extension-touchpad-indicator.src: W: spelling-error %description -l en_US minimalistic -> minimalist, minimalism, animistic
gnome-shell-extension-touchpad-indicator.src: W: spelling-error %description -l en_US trackpoint -> track point, track-point, checkpoint
gnome-shell-extension-touchpad-indicator.src: W: spelling-error %description -l en_US startup -> start up, start-up, upstart

Personally, I wouldn't bother.


All of the automatic checks ran by fedora-review were OK. Same for the things I had to check by myself, except for one thing: the spec file states that the license is GPLv2+, but licensecheck reported that convenience.js is under the BSD license with the "no advertising" clause. 

https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#3ClauseBSD

According to

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios

you should change the license line in your spec file to:

License:        GPLv2+ and BSD

and it should be preceded by an explanatory comment, like this one:

# The entire source code is GPLv2+ except convenience.js, which is BSD

See the link above for more, it also states that the %files section should contain a breakdown of the files grouped by license.

In the few tens of spec files I have read, not many people make use of "c=%{commit}" or "d=%{_datadir}/gnome-shell/extensions/%{uuid}", they just repeat things over and over again. Perhaps my sample is not statistically significant and I really have no idea if seasoned packagers would consider it elegant or lazy (I like it though).

Well done and I hope a proven packager will review your package as soon as possible.

Comment 2 Ville Skyttä 2015-04-11 11:29:39 UTC
Good job with the review, thanks. License tag fixed and comment added:

https://scop.fedorapeople.org/packages/gnome-shell-extension-touchpad-indicator.spec
https://scop.fedorapeople.org/packages/gnome-shell-extension-touchpad-indicator-0-2.20150221gitf2f30f4.fc23.src.rpm

The comment and %files section breakdown etc are in the "...actual implementation of this is left to the maintainer. Some suggested implementations include..." category, so I think the comment is enough (they're suggested, not mandatory, and not all of them need to be done anyway).

Comment 3 Alexander Ploumistos 2015-04-11 16:40:09 UTC
(In reply to Ville Skyttä from comment #2)
> The comment and %files section breakdown etc are in the "...actual
> implementation of this is left to the maintainer. Some suggested
> implementations include..." category, so I think the comment is enough
> (they're suggested, not mandatory, and not all of them need to be done
> anyway).

Fine by me, no need for nitpicking.

Have you by any chance tested the extension with libinput?

https://fedoraproject.org/wiki/Changes/LibinputForXorg

Comment 4 Ville Skyttä 2015-04-14 06:00:08 UTC
(In reply to Alexander Ploumistos from comment #3)
> Have you by any chance tested the extension with libinput?

No. My laptop is still at F-20, not sure if that stuff would be doable with it.

Comment 5 Ville Skyttä 2015-06-10 06:54:38 UTC
https://scop.fedorapeople.org/packages/gnome-shell-extension-touchpad-indicator-0-2.20150602git1b7c3a0.fc22.src.rpm
https://scop.fedorapeople.org/packages/gnome-shell-extension-touchpad-indicator.spec

- Update to current git for gnome-shell 3.16 support

I'm now running this with libinput on F-22 and it appears to work ok. But then again I also ran without this and did not encounter the accidental touchpad hits nearly as badly as with F-20 so I'm not sure if I'm going to need this package after all.

This upstream commit does not sound too promising either:
https://github.com/orangeshirt/gnome-shell-extension-touchpad-indicator/commit/b0c6802edf66a6d4850adc40e6e6b2aa5b4ba292
"... but I don't support and develop the extension by myself, perhaps there ist somebody which will help you."

Comment 6 Ville Skyttä 2015-07-26 11:09:13 UTC
Looks like I'm not going to be needing this package myself, therefore I'm not going to maintain it either.

I'll keep this review request open for a bit in case someone else will take over, will drop it and delete the files in a few weeks if not.

Comment 7 Upstream Release Monitoring 2015-10-23 19:12:17 UTC
pbrobinson's scratch build of gnome-dvb-daemon?#451a118009234eac9e3f06f465b00cf93478a5d5 for git://pkgs.fedoraproject.org/gnome-dvb-daemon?#451a118009234eac9e3f06f465b00cf93478a5d5 and rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11560895

Comment 8 Ville Skyttä 2015-11-03 19:05:50 UTC
Dropped and deleted.


Note You need to log in before you can comment on or make changes to this bug.