Bug 2023118 - Review Request: python-sphinx-markdown-tables - Sphinx extension for rendering markdown tables
Summary: Review Request: python-sphinx-markdown-tables - Sphinx extension for renderin...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-11-14 21:25 UTC by Sandro Mani
Modified: 2021-11-16 17:22 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2021-11-16 17:22:06 UTC
Type: ---
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)

Description Sandro Mani 2021-11-14 21:25:35 UTC
Spec URL: https://smani.fedorapeople.org/review/python-sphinx-markdown-tables.spec
SRPM URL: https://smani.fedorapeople.org/review/python-sphinx-markdown-tables-0.0.15-1.fc36.src.rpm
Description: Sphinx extension for rendering markdown tables
Fedora Account System Username: smani

Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=78873808

Comment 1 Maxwell G 2021-11-16 04:09:17 UTC
Here are a couple informal suggestions:

> %global pkg_name sphinx-markdown-tables

You should replace `pkg_name` with `srcname`; that name is more descriptive and many other packages use that.

> %description
> A Sphinx extension for rendering tables written in markdown.
> [...]
> %description -n python3-%{pkg_name}
> A Sphinx extension for rendering tables written in markdown.

I recommend wrapping the description in a macro instead of repeating it twice. Here[1,2] are two examples.

> %py3_check_import sphinx_markdown_tables
If you want, you can replace this with %pyproject_check_import`[3]. It doesn't require any arguments; it imports all public modules found by the `%pyproject_save_files` macro.

Maxwell

[1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_empty_spec_file
[2]: https://src.fedoraproject.org/rpms/yt-dlp/blob/rawhide/f/yt-dlp.spec#_24
[3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_test_macros

Comment 2 Zbigniew Jędrzejewski-Szmek 2021-11-16 08:41:53 UTC
+ package name is OK
+ latest version
+ license is acceptable for Fedora (GPLv3)
+ license is specified correctly
+ the standard python spec file template was used, so not much to say here
+ BR/R/P look OK
+ fedora-review finds no issues
+ rpmlint:
python3-sphinx-markdown-tables.noarch: W: spurious-executable-perm /usr/share/doc/python3-sphinx-markdown-tables/README.md
python3-sphinx-markdown-tables.noarch: E: wrong-script-end-of-line-encoding /usr/share/doc/python3-sphinx-markdown-tables/README.md
python3-sphinx-markdown-tables.noarch: E: script-without-shebang /usr/share/licenses/python3-sphinx-markdown-tables/LICENSE
python3-sphinx-markdown-tables.noarch: E: wrong-script-end-of-line-encoding /usr/share/licenses/python3-sphinx-markdown-tables/LICENSE

Hmm, I'm confused by this. The file doesn't have +x in the tarball. This seems
to be some bug in %pyproject_install. Miro, comments on this?

Package is APPROVED.

Comment 3 Miro Hrončok 2021-11-16 10:39:07 UTC
The files are installed by:

%license LICENSE
%doc README.md

Not by %pyproject_install. The only way this was caused by %pyproject_* is if the macros actually changed the perms of the local files. Upstream code might do that, but the macros don't. I will investigate the package.

Comment 4 Miro Hrončok 2021-11-16 10:43:02 UTC
[SPECS]$ rpmbuild -bp python-sphinx-markdown-tables.spec 
setting SOURCE_DATE_EPOCH=1636848000
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.wVUQFg
+ umask 022
+ cd /home/churchyard/rpmbuild/BUILD
+ cd /home/churchyard/rpmbuild/BUILD
+ rm -rf sphinx-markdown-tables-0.0.15
+ /usr/bin/gzip -dc /home/churchyard/rpmbuild/SOURCES/sphinx-markdown-tables-0.0.15.tar.gz
+ /usr/bin/tar -xof -
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd sphinx-markdown-tables-0.0.15
+ /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ /usr/bin/cat /home/churchyard/rpmbuild/SOURCES/sphinx-markdown-tables_license.patch
+ /usr/bin/patch -p1 -s --fuzz=0 --no-backup-if-mismatch
+ RPM_EC=0
++ jobs -p
+ exit 0
[SPECS]$ ls -l ../BUILD/sphinx-markdown-tables-0.0.15/LICENSE 
.rwxr-xr-x@ 35k churchyard 31 May  2018 ../BUILD/sphinx-markdown-tables-0.0.15/LICENSE
[SPECS]$ ls -l ../BUILD/sphinx-markdown-tables-0.0.15/README.md 
.rwxr-xr-x@ 1.4k churchyard  5 Jun  2018 ../BUILD/sphinx-markdown-tables-0.0.15/README.md


Also:

[SPECS]$ tar vtf ../SOURCES/sphinx-markdown-tables-0.0.15.tar.gz | grep LICENSE
-rwxrwxrwx ryan/ryan     35821 2018-05-31 05:05 sphinx-markdown-tables-0.0.15/LICENSE
[SPECS]$ tar vtf ../SOURCES/sphinx-markdown-tables-0.0.15.tar.gz | grep README.md
-rwxrwxrwx ryan/ryan      1369 2018-06-05 02:01 sphinx-markdown-tables-0.0.15/README.md

The files are executable.

Comment 5 Gwyn Ciesla 2021-11-16 16:01:57 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-sphinx-markdown-tables

Comment 6 Sandro Mani 2021-11-16 17:22:06 UTC
LICENSE perms fixed. Thanks for the review!


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