Bug 2033730 - Review Request: python-colored-traceback - a library to color exception traces
Summary: Review Request: python-colored-traceback - a library to color exception traces
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Karolina Surma
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 2033702 2035742
TreeView+ depends on / blocked
 
Reported: 2021-12-17 17:36 UTC by W. Michael Petullo
Modified: 2022-01-13 01:05 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-01-13 01:05:26 UTC
Type: ---
ksurma: fedora-review+


Attachments (Terms of Use)

Description W. Michael Petullo 2021-12-17 17:36:25 UTC
Spec URL: https://www.flyn.org/SRPMS/python-colored-traceback.spec
SRPM URL: https://www.flyn.org/SRPMS/python-colored-traceback-0.3.0-1.fc35.src.rpm
Description: A library to color exception traces
Fedora Account System Username: mikep

Comment 1 Karolina Surma 2021-12-23 07:25:32 UTC
Hi,

I consider this a blocker:

%check section is entirely missing. 
According to the guidelines, if it's impossible to run the upstream test suite during the package build, at least a smoke import test must be run in %check. 
See info on %py3_check_import: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#py3_check_import

%python_provide is deprecated, you can safely remove the whole line: %{?python_provide:%python_provide python3-%{srcname}}.


A few non-blocking nitpicks:

Take a look at the new Python Packaging Guidelines (https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_distro_wide_guidelines) which promote pyproject-rpm-macros instead of the py3_build and py3_install. They can also take care of the file list in the package. 
The new macros are actively supported and developed. That's not a blocker, the use of the older macros is still valid.

Source0: You can use a convenient macro  %pypi_source which will resolve to the same url as in your specfile.

From fedora-review: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.

Think about not using %srcname macro, the name is unlikely to change and it's quite inconvenient to e.g. use the upstream URL when it contains macro value. 
I'd use the string `colored-traceback` directly.

Comment 2 Miro Hrončok 2021-12-23 14:09:19 UTC
There's also https://github.com/staticshock/colored-traceback.py/blob/master/test.py

Comment 3 W. Michael Petullo 2021-12-23 18:48:26 UTC
Thank you!

Changes:

 - Add %check smoke test
 - Remove %python_provide
 - Use %pypi_source
 - Remove %srcname. Good point---I have seen arguments for an against. I like your argument.

I will contact upstream about the licensing file. I plan to adopt pyproject-rpm-macros across all of my Python projects later.

Spec URL: https://www.flyn.org/SRPMS/python-colored-traceback.spec
SRPM URL: https://www.flyn.org/SRPMS/python-colored-traceback-0.3.0-1.fc35.src.rpm
Description: A library to color exception traces
Fedora Account System Username: mikep

Comment 4 W. Michael Petullo 2021-12-23 18:57:44 UTC
Regarding license file, another developer has reported. See https://github.com/staticshock/colored-traceback.py/pull/17.

Comment 5 Karolina Surma 2022-01-03 07:54:39 UTC
Thank you for addressing my remarks. 
A nice catch from Miro that there's actually an upstream test which could be run in the build. It would be even better than the smoke test. 
In case of this package I don't find it blocking.

fedora-review has passed. 

Package is APPROVED.

Comment 6 Gwyn Ciesla 2022-01-04 16:28:59 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-colored-traceback

Comment 7 Fedora Update System 2022-01-04 17:05:06 UTC
FEDORA-2022-e3f50b7638 has been submitted as an update to Fedora 35. https://bodhi.fedoraproject.org/updates/FEDORA-2022-e3f50b7638

Comment 8 W. Michael Petullo 2022-01-04 17:05:52 UTC
Thank you!

Comment 9 Fedora Update System 2022-01-05 01:15:04 UTC
FEDORA-2022-e3f50b7638 has been pushed to the Fedora 35 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2022-e3f50b7638 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-e3f50b7638

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 10 Fedora Update System 2022-01-13 01:05:26 UTC
FEDORA-2022-e3f50b7638 has been pushed to the Fedora 35 stable repository.
If problem still persists, please make note of it in this bug report.


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