[Bug 1225231] Review Request: light-locker-settings - Just a simple settings dialog for light-locker

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

 



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



--- Comment #5 from Raphael Groner <projects.rg@xxxxxxxx> ---
Spec URL:
https://raphgro.fedorapeople.org/review/locker/light-locker-settings.spec
SRPM URL:
https://raphgro.fedorapeople.org/review/locker/light-locker-settings-1.5.0-2.fc22.src.rpm

Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=10426299

* Tue Jul 21 2015 Raphael Groner <> - 1.5.0-2
- include python3 patch
- include usability patch (upstream bug 1463476)
- fix execution bits
- fix installation of documentation

(In reply to Yajo from comment #3)
> This is an unofficial review.
> 
> At a first glance, I see the following errors:
> 
> > License:        GPLv3+
> Reading the source and the project URL, it's only GPLv3.

>From where do you get that it is GPLv3 only? COPYING file says it's GPLv3+ and
that any further version of the GPL license is also possible, I don't see
anything to forbid that.

> > %setup -q
> You might prefer %autosetup unless you target EPEL<7. See
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#.
> 25autosetup

My understanding is that both are possible as alternatives. I am not convinced
about applying all patches automatically cause they need proper formatting
then, at least for the pathes and you won't be able to use individual patch -p
options.

> > BuildRequires: python3
> 
> Should be python3-devel. See
> https://fedoraproject.org/wiki/Packaging:Python#BuildRequires

Fixed. Found also a python3 patch in upstream tracker.

> > # configure macro does not work
> 
> IMHO A little explanation would prevent future developers to lose time
> wondering why.

Done.

> > * Tue May 26 2015 Raphael Groner <projects.rg@xxxxxxxx>
> > -
> 
> Please fill this, also with the version stuff.

Fixed.

> You might want to see the rpmlint errors too:
> 
> Checking: light-locker-settings-1.5.0-1.fc21.x86_64.rpm
>           light-locker-settings-1.5.0-1.fc21.src.rpm
> light-locker-settings.x86_64: W: spelling-error %description -l en_US
> screensaver -> screen saver, screen-saver, screens aver

Fixed.

> light-locker-settings.x86_64: W: no-version-in-last-changelog

Fixed, see above.

> light-locker-settings.x86_64: E: no-binary

??

> light-locker-settings.x86_64: E: script-without-shebang
> /usr/share/light-locker-settings/light-locker-settings/light-locker-settings.
> glade

Propably due to wrongly set execution bits. Ignore for now.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

> light-locker-settings.x86_64: W: dangling-symlink
> /usr/share/light-locker-settings/locale /usr/share/locale

Wrong path according to FHS. Not sure if patchable in upstream Makefile:
cp -rf locale $(DESTDIR)/$(PREFIX)/share
ln -sf $(PREFIX)/share/locale $(DESTDIR)/$(PREFIX)/share/$(APPNAME)/locale

> light-locker-settings.x86_64: W: spurious-executable-perm
> /usr/share/doc/light-locker-settings/COPYING
> light-locker-settings.x86_64: W: spurious-executable-perm
> /usr/share/doc/light-locker-settings/INSTALL

Propably due to wrongly set execution bits. Ignore for now.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

> light-locker-settings.x86_64: W: no-manual-page-for-binary
> light-locker-settings

Not sure if we've to provide a manpage.

> light-locker-settings.x86_64: W: install-file-in-docs
> /usr/share/doc/light-locker-settings/INSTALL

Fixed.

> light-locker-settings.src: W: spelling-error %description -l en_US
> screensaver -> screen saver, screen-saver, screens aver

Fixed.

> light-locker-settings.src: W: no-version-in-last-changelog

Fixed, see above.

> light-locker-settings.src:36: W: configure-without-libdir-spec
> 2 packages and 0 specfiles checked; 2 errors, 10 warnings.

> Also, you might want to create appdata files, or at least notify upstream a
> bug about it. See
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#AppData_files

This is no MUST but clearly SHOULD. I'll follow upstream features.
https://fedoraproject.org/wiki/Packaging:AppData

-- 
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
https://admin.fedoraproject.org/mailman/listinfo/package-review




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