Bug 1529167

Summary: Review Request: python-parso - Parser that supports error recovery and round-trip parsing
Product: [Fedora] Fedora Reporter: Carl George <carlwgeorge>
Component: Package ReviewAssignee: Björn Esser (besser82) <besser82>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: besser82, package-review
Target Milestone: ---Flags: besser82: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-01-02 16:21:53 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1493798    

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