Bug 2033481 - Review Request: python-pywaffle - Python package for plotting waffle charts
Summary: Review Request: python-pywaffle - Python package for plotting waffle charts
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 2011908 (view as bug list)
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2021-12-17 01:20 UTC by Matthew Miller
Modified: 2024-01-10 00:45 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-01-10 00:45:32 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Matthew Miller 2021-12-17 01:20:10 UTC
Spec URL: https://mattdm.org/misc/fedora/pywaffle/python-pywaffle.spec
SRPM URL: https://mattdm.org/misc/fedora/pywaffle/python-pywaffle-0.6.3-1.20210728git0857bae.fc35.mattdm.src.rpm

Description:
PyWaffle is an open source, MIT-licensed Python package for plotting waffle
charts. It provides a `Figure` constructor class `Waffle`, which can be
passed to `matplotlib.pyplot.figure` to generate a matplotlib `Figure`
object. Note that the Fedora package has been modified to use the system
version of FontAwesome.

Fedora Account System Username: mattdm

Comment 1 Matthew Miller 2021-12-17 01:21:25 UTC
*** Bug 2011908 has been marked as a duplicate of this bug. ***

Comment 2 Zbigniew Jędrzejewski-Szmek 2021-12-18 10:52:07 UTC
There is no need to stuff the snapshot information into the release tag.
Do something like this instead:

--- python-pywaffle.spec
+++ python-pywaffle.spec
@@ -2 +1,0 @@
-%global pypi_version 0.6.3
@@ -4,0 +4 @@
+%global commitdate 20210728
@@ -9,2 +9,2 @@ Name:           python-%{pypi_name}
-Version:        %{pypi_version}
-Release:        %{autorelease -s 20210728git%{shortcommit}}
+Version:        0.6.3^%{commitdate}git%{shortcommit}
+Release:        %autorelease

+ package name is OK
+ license is acceptable for Fedora (MIT)
+ license is specified correctly
+ builds and installs OK
+ BR/R/P look OK
+ fedora-review finds no issues (except some false positives)
+ rpmlint:

python3-pywaffle.noarch: W: spelling-error %description -l en_US matplotlib -> diplomatist
python3-pywaffle.noarch: W: spelling-error %description -l en_US pyplot -> plot
python3-pywaffle.noarch: E: non-executable-script /usr/lib/python3.9/site-packages/pywaffle/__init__.py 644 /usr/bin/python 
python3-pywaffle.noarch: E: non-executable-script /usr/lib/python3.9/site-packages/pywaffle/fontawesome/__init__.py 644 /usr/bin/python 
python3-pywaffle.noarch: E: non-executable-script /usr/lib/python3.9/site-packages/pywaffle/waffle.py 644 /usr/bin/python 
1 packages and 0 specfiles checked; 3 errors, 2 warnings.
Those can all be ignored.

I'm not keen on all those macros. It's a matter of preference, but let me just say that github repo
naming is case-insensitive, so you could just use "pywaffle" everywhere and everything should work…

Package is APPROVED. Please consider using the more modern version-release split as suggested above.

Comment 3 Matthew Miller 2021-12-18 16:45:37 UTC
Thanks for the review!


> There is no need to stuff the snapshot information into the release tag.


Okay. We should probably update https://docs.pagure.org/Fedora-Infra.rpmautospec/autorelease.html, which is where I got the idea that that's now a good place to put it. I'll change that, and look at simplifying the macros. Thanks again!

Comment 4 Matthew Miller 2021-12-18 16:50:18 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> I'm not keen on all those macros. It's a matter of preference, but let me
> just say that github repo
> naming is case-insensitive, so you could just use "pywaffle" everywhere and
> everything should work…

Unfortunately no, because the PyWaffle variant is still used in the directory name in the github tarball.

Comment 6 Miro Hrončok 2021-12-19 11:23:02 UTC
Several comments, nothing really bad, just nit-picks:


> %{?python_provide:%python_provide python3-%{pypi_name}}

This is deprecated and should not be used. It more or less does nothing.



> python3 pywaffle/scripts/get_fontawesome.py

%{python3} should be used, so it is possible to build the spec file with a different Python version (unlikely to be needed, but at least consistent).



> %global pypi_name pywaffle
> %global pypi_version 0.6.3
> %global githubname PyWaffle

I highly recommend not doing this. It makes the spec file harder to read and the benefit is minimal, it is not like I could take the spec file, copy it, change the values and use it without changes for another package.
Using the literal values in the spec file makes it easier to understand.



> %license LICENSE

This is not needed, as the LICENSE file is listed and marked as %license in %{pyproject_files}:

$ rpm -ql --licensefiles -p python3-pywaffle-0.6.3-1.20210728git0857bae.fc36.noarch.rpm 
/usr/lib/python3.10/site-packages/pywaffle-0.6.3.dist-info/LICENSE
/usr/share/licenses/python3-pywaffle/LICENSE


(On Fedora 34 or EPEL 9 the file is installed, but not marked as %license.)

Comment 7 Matthew Miller 2021-12-19 18:30:39 UTC
(In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
> >  We should probably update https://docs.pagure.org/Fedora-Infra.rpmautospec/autorelease.html
> https://pagure.io/fedora-infra/rpmautospec/pull-request/222

Ah, so... pending infra docs PR. I'll see what I can do to push that along. Thanks!


(In reply to Miro Hrončok from comment #6)
> Several comments, nothing really bad, just nit-picks:

Cool, thanks. I'll respond with my naïve comments in the related thread on our python-devel list.

Comment 8 Package Review 2022-12-10 16:52:24 UTC
Package was never imported, resetting ticket status.

Comment 9 Package Review 2023-12-11 00:45:22 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

Comment 10 Package Review 2024-01-10 00:45:32 UTC
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.


Note You need to log in before you can comment on or make changes to this bug.