Bug 2184588
| Summary: | Review Request: python-flask-mailman - Porting Django's email implementation to your Flask applications | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Sandro Mani <manisandro> |
| Component: | Package Review | Assignee: | Felix Wang <topazus> |
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | mhroncok, package-review, snamila, sudar2, topazus |
| Target Milestone: | --- | Flags: | topazus:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | https://github.com/waynerv/flask-mailman | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2023-04-11 22:13:23 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Attachments: | |||
|
Description
Sandro Mani
2023-04-05 06:05:47 UTC
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. |