Bug 2160509 - Review Request: python-colored - Library for color and formatting in terminal
Summary: Review Request: python-colored - Library for color and formatting in terminal
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Carl George 🤠
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-01-12 16:22 UTC by Jonathan Wright
Modified: 2024-08-09 19:33 UTC (History)
2 users (show)

Fixed In Version: python-colored-1.4.4-1.fc38
Clone Of:
Environment:
Last Closed: 2023-09-01 21:49:16 UTC
Type: ---
Embargoed:
carl: fedora-review+


Attachments (Terms of Use)

Description Jonathan Wright 2023-01-12 16:22:08 UTC
Spec URL: https://jonathan@almalinux.org.fedorapeople.org/python-colored.spec
SRPM URL: https://jonathan@almalinux.org.fedorapeople.org/python-colored-1.4.4-1.fc38.src.rpm

Description:
Very simple Python library for color and formatting in terminal.
Collection of color codes and names for 256 color terminal setups.

Fedora Account System Username: jonathan

Comment 1 Jonathan Wright 2023-01-12 16:22:11 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=96058017

Comment 2 Jakub Kadlčík 2023-01-12 16:25:01 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5224090
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2160509-python-colored/srpm-builds/05224090/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

Comment 3 Jonathan Wright 2023-01-12 16:30:03 UTC
copr failure is unrelated.  It is due to fedora-create-review putting the wrong FAS username in this ticket.

Comment 5 Jakub Kadlčík 2023-01-19 18:03:55 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5255607
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2160509-python-colored/fedora-rawhide-x86_64/05255607-python-colored/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

Comment 6 Fedora Update System 2023-01-19 18:06:34 UTC
FEDORA-2023-30df91a05c has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-30df91a05c

Comment 7 Fedora Update System 2023-01-19 20:54:53 UTC
FEDORA-2023-30df91a05c has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 8 Carl George 🤠 2023-01-19 22:21:07 UTC
Optional suggestion, use the PyPI tarball instead of the GitLab one.  This is not required by the guidelines, but I believe most Python packagers tend to stick with PyPI tarballs unless it is missing tests or other necessary files.

-Source:         https://gitlab.com/dslackw/colored/-/archive/%{version}/colored-%{version}.tar.gz
+Source:         %{pypi_source colored}

https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_source_files_from_pypi

================================================================================

There is a build requirement for pytest, but it is not used to run the test, so it should be removed.

-# for tests
-BuildRequires:  python3-pytest

================================================================================

Upstream's TravisCI config claims to run nosetests, but none of the files in the tests directory contain valid unit tests.  They instead appear to be scripts to run manually and visually compare the color output.  I can see the only thing is %check is an import test.  I agree with that approach, but it would also be useful to add a comment about why the upstream tests can't be run.

================================================================================

The LICENSE.txt file is properly picked up by the Python metadata, so listing it explicitly in %files produces a duplicate.  The explicit listing should be removed to avoid this.

-%license LICENSE.txt

Comment 9 Jonathan Wright 2023-01-23 20:06:45 UTC
(In reply to Carl George 🤠 from comment #8)
> Optional suggestion, use the PyPI tarball instead of the GitLab one.  This
> is not required by the guidelines, but I believe most Python packagers tend
> to stick with PyPI tarballs unless it is missing tests or other necessary
> files.
> 
> -Source:        
> https://gitlab.com/dslackw/colored/-/archive/%{version}/colored-%{version}.
> tar.gz
> +Source:         %{pypi_source colored}
> 
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
> #_source_files_from_pypi
> 

I tend to go straight for GH to avoid such issues altogether.  I'd prefer to stick to GH but will happily change it of course if policy ever changes (or later on it seems like upstream maintains releases on PyPi better).

> 
> There is a build requirement for pytest, but it is not used to run the test,
> so it should be removed.
> 
> -# for tests
> -BuildRequires:  python3-pytest

Already removed in my local copy.  Looks like I uploaded a slightly outdated one here by mistake.

> Upstream's TravisCI config claims to run nosetests, but none of the files in
> the tests directory contain valid unit tests.  They instead appear to be
> scripts to run manually and visually compare the color output.  I can see
> the only thing is %check is an import test.  I agree with that approach, but
> it would also be useful to add a comment about why the upstream tests can't
> be run.

Added a note.

> The LICENSE.txt file is properly picked up by the Python metadata, so
> listing it explicitly in %files produces a duplicate.  The explicit listing
> should be removed to avoid this.
> 
> -%license LICENSE.txt

Good catch, fixed in my local copy.

Comment 10 Carl George 🤠 2023-01-23 20:15:19 UTC
Package is approved.

Comment 11 Fedora Admin user for bugzilla script actions 2023-01-23 20:18:09 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-colored

Comment 13 Fedora Update System 2024-08-09 19:30:24 UTC
FEDORA-EPEL-2024-c4b2008a49 (python-cloudpickle-3.0.0-8.el10_0) has been submitted as an update to Fedora EPEL 10.0.
https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2024-c4b2008a49

Comment 14 Fedora Update System 2024-08-09 19:33:11 UTC
FEDORA-EPEL-2024-c4b2008a49 (python-cloudpickle-3.0.0-8.el10_0) has been pushed to the Fedora EPEL 10.0 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.