Bug 1529167 - Review Request: python-parso - Parser that supports error recovery and round-trip parsing
Summary: Review Request: python-parso - Parser that supports error recovery and round-...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Björn Esser (besser82)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1493798
TreeView+ depends on / blocked
 
Reported: 2017-12-26 20:07 UTC by Carl George
Modified: 2018-01-02 16:21 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2018-01-02 16:21:53 UTC
Type: ---
Embargoed:
besser82: fedora-review+


Attachments (Terms of Use)

Description Carl George 2017-12-26 20:07:25 UTC
Spec URL: https://carlwgeorge.fedorapeople.org/python-parso.spec
SRPM URL: https://carlwgeorge.fedorapeople.org/python-parso-0.1.1-1.fc28.src.rpm
COPR: https://copr.fedorainfracloud.org/coprs/carlwgeorge/python-parso/
Description:
Parso is a Python parser that supports error recovery and round-trip parsing
for different Python versions (in multiple Python versions). Parso is also able
to list multiple syntax errors in your python file.  Parso has been
battle-tested by jedi. It was pulled out of jedi to be useful for other
projects as well.  Parso consists of a small API to parse Python and analyse
the syntax tree.
Fedora Account System Username: carlwgeorge

Comment 1 Björn Esser (besser82) 2017-12-29 17:47:50 UTC
Package looks good to me, but `%{python3_sitelib}/*`…  You shoudn't greedy-glob there, since you might own the `__pycache__` directory, which is owned by python3 itself.

Please fix `%{undefined rhel}` ---> `%{!?rhel}` during import!

Package APPROVED!

Comment 2 Björn Esser (besser82) 2017-12-29 17:52:56 UTC
Maybe you could have a look at one of my packages waiting for review in exchange?

https://bugzilla.redhat.com/show_bug.cgi?id=1529352
https://bugzilla.redhat.com/show_bug.cgi?id=1529593
https://bugzilla.redhat.com/show_bug.cgi?id=1529705

Comment 3 Carl George 2017-12-29 19:30:28 UTC
> You shoudn't greedy-glob there, since you might own the `__pycache__` directory

I don't necessarily disagree, but I duplicated that part from the example spec file in the guidelines.  How do we go about getting the guidelines updated?

https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file

> Please fix `%{undefined rhel}` ---> `%{!?rhel}` during import!

What is the issue here?  The undefined macro exists to make that type of condition more readable.  It's built into RPM (defined in /usr/lib/rpm/macros) and exists as far back as RHEL 5 (perhaps even further).

I'll be happy to review your packages, but it might have to wait until next week unless I can find some time this afternoon.

Comment 4 Björn Esser (besser82) 2017-12-29 19:50:25 UTC
(In reply to Carl George from comment #3)
> > You shoudn't greedy-glob there, since you might own the `__pycache__` directory
> 
> I don't necessarily disagree, but I duplicated that part from the example
> spec file in the guidelines.  How do we go about getting the guidelines
> updated?
> 
> https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file

Well, the guidelines were written in times before Python 3.4 (or 3.3?), which introduced the `__pycache__` dir…  We can try to get them updated, but FPC seems to be pretty unresponsive in the last time…  If I find the time during the naxt days, I'll write a proposal for that.


> > Please fix `%{undefined rhel}` ---> `%{!?rhel}` during import!
> 
> What is the issue here?  The undefined macro exists to make that type of
> condition more readable.  It's built into RPM (defined in
> /usr/lib/rpm/macros) and exists as far back as RHEL 5 (perhaps even further).

Ohh…  I didn't know that existed.  I've said nothing then.  ;)


> I'll be happy to review your packages, but it might have to wait until next
> week unless I can find some time this afternoon.

Yay!  =)

Comment 5 Björn Esser (besser82) 2017-12-29 20:04:23 UTC
There is a new package waiting for review, since the others have already been done in the meantime…

https://bugzilla.redhat.com/show_bug.cgi?id=1529758

Comment 6 Gwyn Ciesla 2018-01-02 14:47:26 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/python-parso


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