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

Otto Urpelainen <otu...@iki.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review?
                   |                            |needinfo?(ti.eugene@gmail.c
                   |                            |om)



--- Comment #15 from Otto Urpelainen <otu...@iki.fi> ---
I have now made a complete review. Findings:

1. I think the License should be "(MIT and LGPLv3 and BSD)". The licensing
guidelines are not crystal clear on this, but I read them as a) no parenthesis:
multiple files, each with one of the listed licenses, b) with parenthesis: one
file containing parts with each of the licenses.

2. All these licenses require including a copy of the license when distributing
the code. So, either the LICENSE file needs to be updated to contain licenses
for everything that is bundled, or some other way invented to get the all the
licenses and copyright notices included in %license. I think a pull request is
needed upstream to get this right.

3. Annoyingly, AppStream metadata also contains a field for liceses,
project_license. So qvge.appdata.xml must also be updated to list the actual
license that has now been determined. That goes naturally to the same pull
request where licensing is otherwise made compatible with Fedora packaging.

4. Add "BuildRequires: make". This should be explictly listed nowadays if make
is used.

5. fedora-review complains: "Note: Directories without known owners:
/usr/share/mime/packages, /usr/share/mime". Add directories must have assigned
ownership, in this case the correct solution would be to add "Requires:
shared-mime-info", which is a "filesystem package" owning those directories.
Reference:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_file_and_directory_ownership

6. There is no man page, you should get in contact with the upstream about
adding such. Pull request is best, but an issue will also do as usual.
Reference: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_manpages

7. Since running as many tests as possible in %check is recommended, perhaps
add a comment line there explaining that upstream does not provide any?
Reference:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_test_suites

I am confident in my review, but if Ben still have some comments, they are
welcome of course.


-- 
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@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to