[Bug 1020839] Review Request: fedora-gooey-karma - GUI tool for adding karma to Bodhi system

[bugzilla at redhat.com]

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



--- Comment #11 from Branislav Blaškovič <bblaskov at redhat.com> ---
> I might change the Source0 to
> https://github.com/blaskovic/fedora-gooey-karma/archive/%{name}-%{version}.
> tar.gz , as that's what the file is actually called once it gets through
> github's magic.

I would leave it as is. It's creating (after unpacking)
fedora-gooey-karma-fedora-gooey-karma-0.1 folder when used with your url.

> fedora-gooey-karma.noarch: E: script-without-shebang
> /usr/share/fedora-gooey-karma/mainwindow_gui.py
> if that script isn't meant to be called directly it should not have
> executable permissions; if it is, then it should have a shebang.

Fixed. This script should not be executable.

$ rpmlint fedora-gooey-karma-0.1-1.fc20.noarch.rpm
fedora-gooey-karma.noarch: W: no-manual-page-for-binary fedora-gooey-karma
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint fedora-gooey-karma-0.1-1.fc20.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

> Why is the .desktop file marked "OnlyShowIn=GNOME;" ? Is there some reason
> it won't work on other desktops?

Removed. I recycled that .desktop file from some app which had this. My bad.

> The app not having an icon is kinda bad, you could ask if the art team could
> provide you with one.

I will try to create one or contact somebody with some art-skill.

> MUST: If (and only if) the source package includes the text of the
> license(s) in its own file, then that file, containing the text of the
> license(s) for the package must be included in %doc - this is not done, the
> package includes a COPYING file (GPLv3) but it is not packaged.

Fixed. It's packaged (via %doc) now.

> MUST: Permissions on files must be set properly. Executables should be set
> with executable permissions, for example. - see note about python file above

Fixed as mentioned above.

> SHOULD: your package should contain man pages for binaries/scripts. If it
> doesn't, work with upstream to add them where they make sense. - there
> aren't any, as noted by rpmlint. But it probably doesn't make sense to have
> one for a simple command/app which probably takes no or few arguments.

I can create a simple man page for this. But it's not required in my opinion
because this app has 0 arguments.

> - spec file mixes $RPM_BUILD_ROOT and %{buildroot}, pick one form only.

I've picked %{buildroot}.

> - %clean section is not needed:
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#.
> 25clean

Removed.

> - %defattr is not needed:
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#File_Permissions

Removed.

> Add leading space when using \ :
> 
> desktop-file-install \
>     --dir=%{buildroot}%{_datadir}/applications \
>     %{buildroot}/%{_datadir}/applications/fedora-gooey-karma.desktop

Fixed.

> - Most of empty lines seems unneeded.

I've removed doubled empty lines.

> - Summary is cryptic, please improve.

Does somebody have some suggestions to make the summary clear?

-- 
You are receiving this mail because:
You are on the CC list for the bug.