[Bug 1073978] Review Request: photocollage - An image assembler with a Gtk GUI

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

 



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



--- Comment #6 from Kalev Lember <kalevlember@xxxxxxxxx> ---
So, you are a new packager. Welcome to Fedora, Adrien! I hope you'll enjoy
this.

As a new packager, you'll need to get sponsored into the packager group. This
is a one time process and it's much easier to get other packages in once you've
cleared this initial step and are part of the packager group.

All packagers are also automatically reviewers, so it's expected that everyone
knows how to perform package reviews. It's common to ask for new people to show
their understanding of the packaging guidelines by asking them to perform one
package review. Could you choose a ticket of your liking from the list in
http://fedoraproject.org/PackageReviewStatus/NEW.html and go through the
package review checklist in
https://fedoraproject.org/wiki/Packaging:ReviewGuidelines and post your
findings in the ticket?


Regarding the package here, I've taken a quick look and it looks nice and
clean. Good work!


One thing I've noticed is that the source files don't have license headers. It
would be great if you could add them upstream as per
https://www.gnu.org/licenses/old-licenses/gpl-2.0.html#SEC4

Fedora is quite strict with legal matters and things that concern licensing. In
the licensing FAQ, there's a long section what to do when there's only a
COPYING file and no license headers:
https://fedoraproject.org/wiki/Licensing:FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F
It's probably easier to just add the license headers and clear any confusion
with that.


The spec file looks largely fine. I have some nitpick comments below:

> License:        GPLv2

Is it GPLv2 or GPLv2+ ? The rpm spec file and the PKG-INFO file seem to
disagree. And no license headers in the .py files to double check this.


> %setup -q -n %{name}-%{version}

Remove the "-n %{name}-%{version}" part, that's the default.


> desktop-file-install --vendor="" \
>         --dir=%{buildroot}%{_datadir}/applications/ \
>         %{buildroot}%{_datadir}/applications/%{name}.desktop

It would be better to use desktop-file-validate here. desktop-file-install is
mostly for if you need to edit the .desktop file (e.g. add or remove a
category), or when you are including an external desktop file that doesn't come
from upstream. None of this applies here.


Also, I see you've done a few changes to the upstream tarball without changing
the version. It's better to bump the version each time you need to roll a new
release. It can be very confusing for other people if a tarball with the same
version gets silently replaced.

http://www.scrye.com/wordpress/nirik/2014/03/11/just-say-no-to-re-releasing-the-same-version-of-software/

-- 
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]