Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec SRPM URL: https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-1.fc39.src.rpm Description: Porting Django's email implementation to your Flask applications Fedora Account System Username: smani
Copr build: https://copr.fedorainfracloud.org/coprs/build/5744517 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05744517-python-flask-mailman/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Using %{pypi_source} without a name argument (i.e. providing the value via %srcname) is deprecated in the new Python guidelines. I strongly suggest not defining %srcname at all. The "# Drop deps" could use some more love (e.g. what kind of deps and why). The second Summary: could be defined via %{summary}. "%license LICENSE" might be redundant. Is it possible to run the tests in %check? If not, please document the reason in a comment.
Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec SRPM URL: https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-2.fc39.src.rpm %changelog * Wed Feb 08 2023 Sandro Mani <manisandro> - 0.3.0-2 - Explicitly specify pypi_source - Remove reduntant license - Run %%tox in %%check - Document patch reason
Created attachment 1955892 [details] The .spec file difference from Copr build 5744517 to 5745670
Copr build: https://copr.fedorainfracloud.org/coprs/build/5745670 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05745670-python-flask-mailman/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
It seems that the pypi source does not contain tests, maybe you can switch to its corresponding tarball from github release. ref: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_source_files_from_pypi
Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec SRPM URL: https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-3.fc39.src.rpm %changelog * Thu Apr 06 2023 Sandro Mani <manisandro> - 0.3.0-3 - Switch to GitHub source
Created attachment 1956066 [details] The .spec file difference from Copr build 5745670 to 5749870
Copr build: https://copr.fedorainfracloud.org/coprs/build/5749870 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05749870-python-flask-mailman/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Sandro Mani from comment #3) > Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec > SRPM URL: > https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-2.fc39.src. > rpm > > %changelog > * Wed Feb 08 2023 Sandro Mani <manisandro> - 0.3.0-2 > - Explicitly specify pypi_source > - Remove reduntant license > - Run %%tox in %%check > - Document patch reason After removing the %license line, it seems that the package does not contain license file. ref: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05749870-python-flask-mailman/python3-flask-mailman-0.3.0-3.fc39.noarch.rpm
(In reply to Felix Wang from comment #10) > (In reply to Sandro Mani from comment #3) > > Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec > > SRPM URL: > > https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-2.fc39.src. > > rpm > > > > %changelog > > * Wed Feb 08 2023 Sandro Mani <manisandro> - 0.3.0-2 > > - Explicitly specify pypi_source > > - Remove reduntant license > > - Run %%tox in %%check > > - Document patch reason > > After removing the %license line, it seems that the package does not contain > license file. > > ref: > https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora- > review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05749870-python- > flask-mailman/python3-flask-mailman-0.3.0-3.fc39.noarch.rpm Sorry for my wrong words and the mistake here, forgot it.
Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec SRPM URL: https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-4.fc39.src.rpm %changelog * Fri Apr 07 2023 Sandro Mani <manisandro> - 0.3.0-4 - Re-add %%license
Created attachment 1956237 [details] The .spec file difference from Copr build 5749870 to 5756969
Copr build: https://copr.fedorainfracloud.org/coprs/build/5756969 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05756969-python-flask-mailman/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Sandro Mani from comment #12) > Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec > SRPM URL: > https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-4.fc39.src. > rpm > > %changelog > * Fri Apr 07 2023 Sandro Mani <manisandro> - 0.3.0-4 > - Re-add %%license I am sorry for my mistake about license issue. You do not need to use %license line to add license. the %pyproject_save_files can add license. ``` ❯ wget https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05749870-python-flask-mailman/python3-flask-mailman-0.3.0-3.fc39.noarch.rpm --2023-04-07 23:01:45-- https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05749870-python-flask-mailman/python3-flask-mailman-0.3.0-3.fc39.noarch.rpm Connecting to 127.0.0.1:7890... connected. Proxy request sent, awaiting response... 200 OK Length: 45441 (44K) [application/x-rpm] Saving to: ‘python3-flask-mailman-0.3.0-3.fc39.noarch.rpm’ python3-flask-mailman-0.3.0-3 100%[=================================================>] 44.38K --.-KB/s in 0.1s 2023-04-07 23:01:46 (419 KB/s) - ‘python3-flask-mailman-0.3.0-3.fc39.noarch.rpm’ saved [45441/45441] ❯ rpm -qlp ./python3-flask-mailman-0.3.0-3.fc39.noarch.rpm | grep LICENSE warning: ./python3-flask-mailman-0.3.0-3.fc39.noarch.rpm: Header V4 RSA/SHA256 Signature, key ID 3aabd587: NOKEY /usr/lib/python3.11/site-packages/flask_mailman-0.3.0.dist-info/LICENSE ``` quoted from Fedora packaging guideline about Python: > %pyproject_save_files MODNAME … > Generate a list of files corresponding to the given importable module(s) and save it as %{pyproject_files}. > Note that README file is not included. The LICENSE file is included when it is specified in the metadata.
Right, I though the license was supposed to be below /usr/share/licenses! Spec URL: https://smani.fedorapeople.org/review/python-flask-mailman.spec SRPM URL: https://smani.fedorapeople.org/review/python-flask-mailman-0.3.0-5.fc39.src.rpm %changelog * Fri Apr 07 2023 Sandro Mani <manisandro> - 0.3.0-5 - Remove reduntant license
Created attachment 1956288 [details] The .spec file difference from Copr build 5756969 to 5757892
Copr build: https://copr.fedorainfracloud.org/coprs/build/5757892 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2184588-python-flask-mailman/fedora-rawhide-x86_64/05757892-python-flask-mailman/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
I see that no assignee at the moment. I take this. You corrected the things that Miro Hrončok said, most of the package seems good to me. As for license issue, I read the documentation again, it said > 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 %license. Since it says 'must be included in %license', so you are right. The additional packaging of license file using %license will be all right, although %pyproject_save_files macro includes the license file. Apologized for my previous misunderstanding，which may give inconvenience to you. But I note that some python-related packages does not use %license (put license under /usr/share/licenses), like python3-pytest, python3-virtualenv. ref: https://packages.fedoraproject.org/pkgs/pytest/python3-pytest/fedora-rawhide.html https://packages.fedoraproject.org/pkgs/python-virtualenv/python3-virtualenv/fedora-rawhide.html Finally, I approved the package.
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-flask-mailman
Btw, Can you help me with the review request of rust-systemstat? ref: https://bugzilla.redhat.com/show_bug.cgi?id=2184899
Thanks for the review.