Bug 1201925

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.

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

(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

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

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

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.

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

Dropped and deleted.