Bug 1288453 - Review Request: python-CommonMark - Python parser for the CommonMark Markdown spec
Summary: Review Request: python-CommonMark - Python parser for the CommonMark Markdown...
Keywords:
Status: CLOSED ERRATA
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: 1288456
TreeView+ depends on / blocked
 
Reported: 2015-12-04 09:42 UTC by Julien Enselme
Modified: 2015-12-15 13:23 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-14 11:51:04 UTC
Type: ---
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)

Description Julien Enselme 2015-12-04 09:42:41 UTC
Spec URL: http://dl.jujens.eu/SPECS/python-CommonMark.spec
SRPM URL: http://dl.jujens.eu/SRPMS/python-CommonMark-0.5.4-1.fc23.src.rpm
Description:
Pure Python port of jgm’s stmd.js, a Markdown parser and renderer for the
CommonMark specification, using only native modules. Once both this project and
the CommonMark specification are stable we will release the first 1.0 version
and attempt to keep up to date with changes in stmd.js.

We are currently at the same development stage (actually a bit ahead because we
have implemented HTML entity conversion and href URL escaping) as stmd.js. Since
Python versions pre-3.4 use outdated (i.e. not HTML5 spec) entity conversion,
I’ve converted the 3.4 implementation into a single file, entitytrans.py which
so far seems to work (all tests pass on 2.7, 3.3, and 3.4).

Fedora Account System Username: jujens

Comment 1 Zbigniew Jędrzejewski-Szmek 2015-12-04 15:16:55 UTC
Issues:
=======
- Large documentation must go in a -doc subpackage. Large could be size
  (~1MB) or number of files.
  Note: Documentation size is 1576960 bytes in 4 files.
  See:
  http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation

I think it would make sense to create a -doc subpackage, Suggested by the python2- and python3- subpackages. Alternatively, you could use a common
doc directory for both packages (using '%global _docdir_fmt %{name}').

Requires
--------
python3-CommonMark (rpmlib, GLIBC filtered):
    /usr/bin/python2  <-------------------------------- !
    python(abi)

python2-CommonMark (rpmlib, GLIBC filtered):
    python(abi)

Comment 2 Julien Enselme 2015-12-04 19:34:20 UTC
SPEC: http://dl.jujens.eu/SPECS/python-CommonMark.spec
SRPM: http://dl.jujens.eu/SRPMS/python-CommonMark-0.5.4-2.fc23.src.rpm


* Fri Dec 4 2015 Julien Enselme <jujens> - 0.5.4-2
- Correct shebang of cmark.py (/usr/bin/python2 -> /usr/bin/python3)
- Add doc packages.

Comment 3 Zbigniew Jędrzejewski-Szmek 2015-12-05 00:54:23 UTC
The point of the -doc subpackage is to avoid duplication to save disk space. It also saves user confusion, because the don't have to look at python2-*-doc first, and then at python3-*-doc first, and realize that the documentation is identical. Files in python2-CommonMark-doc and python2-CommonMark-doc *are* identical, there should be just one -doc package.

You can avoid defining %sum. Just put the summary in the first Summary, and then use %summary in subsequent subpackages. The summary for -doc should be different though.

Please use %{__python3} to refer to /usr/bin/python3 everywhere in the spec file.

You probably don't need to use sed to fix the shebang line. Just move %py2_install below %py3_install.

PYTHONPATH=$(pwd):$PYTHONPATH → PYTHONPATH=$(pwd)
PYTHONPATH is normally unset, so you don't need to preserve the previous one.

Comment 4 Julien Enselme 2015-12-05 09:51:01 UTC
> You can avoid defining %sum. Just put the summary in the first Summary, and then use %summary in subsequent subpackages.

I didn't know that. Is there something similar for description?


> PYTHONPATH is normally unset, so you don't need to preserve the previous one.

It's more an habit to preserve it, but I agree it is not useful.

> You probably don't need to use sed to fix the shebang line. Just move 
%py2_install below %py3_install.

Should the binary use python3 by default? It works, I tested it. That's why I put %py3_install last and corrected the shebang.


* Sat Dec 5 2015 Julien Enselme <jujens> - 0.5.4-3
- Use only one doc package.
- Use %%summary to avoid summary repetition.
- Use %%__python3 macro to fix shebang.


SPEC: http://dl.jujens.eu/SPECS/python-CommonMark.spec
SRPM: http://dl.jujens.eu/SRPMS/python-CommonMark-0.5.4-3.fc23.src.rpm

Comment 5 Zbigniew Jędrzejewski-Szmek 2015-12-05 16:36:15 UTC
(In reply to Julien Enselme from comment #4)
> > You can avoid defining %sum. Just put the summary in the first Summary, and then use %summary in subsequent subpackages.
> 
> I didn't know that. Is there something similar for description?
Nope.

> > PYTHONPATH is normally unset, so you don't need to preserve the previous one.
> 
> It's more an habit to preserve it, but I agree it is not useful.
> 
> > You probably don't need to use sed to fix the shebang line. Just move 
> %py2_install below %py3_install.
> 
> Should the binary use python3 by default? It works, I tested it. That's why
> I put %py3_install last and corrected the shebang.
Oh, OK. Yes, python3 is preferred [https://fedoraproject.org/wiki/Packaging:Python#Executables_in_.2Fusr.2Fbin].


> * Sat Dec 5 2015 Julien Enselme <jujens> - 0.5.4-3
> - Use only one doc package.
> - Use %%summary to avoid summary repetition.
> - Use %%__python3 macro to fix shebang.
> 
> 
> SPEC: http://dl.jujens.eu/SPECS/python-CommonMark.spec
> SRPM: http://dl.jujens.eu/SRPMS/python-CommonMark-0.5.4-3.fc23.src.rpm

- license is OK
- license file is present, %license is used
- latest version
- builds and installs OK
- new python template is used
- python3 is preferred
- requires and provides are OK
- no scriptlets present or needed
- rpmlint says: no-documentation, no-manual-page-for-binary, spelling-error, python-bytecode-without-source. All false positives or ignorable.

You might want to add Suggests: python-CommonMark-doc to the python2- and python3- subpackages.

Package is APPROVED.

Comment 6 Julien Enselme 2015-12-05 17:10:14 UTC
> Suggests: python-CommonMark-doc to the python2- and python3- subpackages.

Indeed, thanks for the suggestion.

Thanks for the review.

Comment 7 Till Maas 2015-12-05 18:52:39 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/python-CommonMark

Comment 8 Fedora Update System 2015-12-05 19:27:29 UTC
python-CommonMark-0.5.4-3.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-288b77d774

Comment 9 Fedora Update System 2015-12-05 19:33:41 UTC
python-CommonMark-0.5.4-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-6cc818a6aa

Comment 10 Fedora Update System 2015-12-06 05:22:03 UTC
python-CommonMark-0.5.4-3.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update python-CommonMark'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-288b77d774

Comment 11 Fedora Update System 2015-12-06 17:20:24 UTC
python-CommonMark-0.5.4-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update python-CommonMark'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-6cc818a6aa

Comment 12 Fedora Update System 2015-12-14 11:51:01 UTC
python-CommonMark-0.5.4-3.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

Comment 13 Fedora Update System 2015-12-15 13:23:44 UTC
python-CommonMark-0.5.4-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, 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.