Bug 987153 - Review Request: python-modernize - Modernizes Python code for eventual Python 3 migration
Review Request: python-modernize - Modernizes Python code for eventual Python...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Meng
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-22 16:18 EDT by Toshio Ernie Kuratomi
Modified: 2013-08-20 21:50 EDT (History)
2 users (show)

See Also:
Fixed In Version: python-modernize-0.2-2.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-08-20 20:09:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
i: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Toshio Ernie Kuratomi 2013-07-22 16:18:18 EDT
Spec URL: http://toshio.fedorapeople.org/packages/python-modernize.spec
SRPM URL: http://toshio.fedorapeople.org/packages/python-modernize-0.2-1.fc17.src.rpm
Description:
This library is a very thin wrapper around lib2to3 to utilize it
to make Python 2 code more modern with the intention of eventually
porting it over to Python 3.

It does not guarantee, but it attempts to spit out a Python 2/3
compatible codebase.  The code that it generates has a runtime
dependency on python-six.

Fedora Account System Username: toshio
Comment 1 Christopher Meng 2013-07-23 00:11:11 EDT
1. Do you think adding BR for python-setuptools is OK?

2. %files

%{python_sitelib}/*

should be

%{python_sitelib}/libmodernize
%{python_sitelib}/%{srcname}-%{version}-py%{python_version}.egg-info

3. Remove these

# sitelib for noarch packages, sitearch for others (remove the unneeded one)
%{!?python_sitelib: %global python_sitelib %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib())")}
%{!?python_sitearch: %global python_sitearch %(%{__python} -c "from distutils.sysconfig import get_python_lib; print(get_python_lib(1))")}

4. No need to

rm -rf %{buildroot}
Comment 2 Toshio Ernie Kuratomi 2013-07-23 14:46:33 EDT
Thanks for reviewing!

Spec URL: http://toshio.fedorapeople.org/packages/python-modernize.spec
SRPM URL: http://toshio.fedorapeople.org/packages/python-modernize-0.2-2.fc17.src.rpm

1. Good catch on the BR for python-setuptools.  Added.

2. That's stylistic.  I prefer the wildcard for everything in the directory.  I know that some people would rather be notified if the files in that directory change but I'd rather have the package build succeed and look at the sources when necessary (when updating a stable release).  [One note: you're supposed to add a trailing slash to directories that you own so that it's obvious to people that you meant to include a directory recursively.  So: %{python_sitelib}/libmodernize/ and %{python_sitelib}/%{srcname}-%{version}-py%{python_version}.egg-info/ are correct)

3 & 4.  Thanks!  yeah, I'm only pushing this back as far as EPEL6 (if that) so those are good changes.  Fixed.
Comment 3 Christopher Meng 2013-07-23 20:45:10 EDT
Well,

Issue 2 is a problem "Unowned directory".

If you use wildcard, you can include all py files but not directory itself, that's why I want you to change.
Comment 4 Toshio Ernie Kuratomi 2013-07-23 21:09:02 EDT
What directory is unowned?
Comment 5 Christopher Meng 2013-07-23 21:16:50 EDT
%{python_sitelib}/libmodernize itself
Comment 6 Toshio Ernie Kuratomi 2013-07-24 01:23:13 EDT
The wildcard should include that.  In testing, it looks owned to me:

$ rpm -qpl python-modernize-0.2-2.fc17.noarch.rpm |egrep 'libmodernize$'
/usr/lib/python2.7/site-packages/libmodernize
Comment 7 Michael Schwendt 2013-07-24 16:49:22 EDT
The '*' wildcard _never_ results in an unowned directory, except if the _parent_ directory (and the parent's parent, and so on) belong into the package. Hence

  %{python_sitelib}/*

includes _anything_ in /usr/lib/python2.7/site-packages/ (and anything includes directories). The guidelines don't comment on that, because that's the trivial case. On the contrary, if it had been

  %{python_sitelib}/libmodernize/*

the "libmodernize" directory would have been unowned:
https://fedoraproject.org/wiki/Packaging:UnownedDirectories#Wildcarding_Files_inside_a_Created_Directory
Comment 8 Toshio Ernie Kuratomi 2013-07-24 19:06:50 EDT
New Package SCM Request
=======================
Package Name: python-modernize
Short Description: Modernizes Python code for eventual Python 3 migration
Owners: toshio
Branches: f17 f18 f19 devel
InitialCC:
Comment 9 Christopher Meng 2013-07-29 20:45:21 EDT
Hmm...You forgot to change the flag...
Comment 10 Gwyn Ciesla 2013-07-30 08:06:26 EDT
Git done (by process-git-requests).
Comment 11 Fedora Update System 2013-07-31 03:50:26 EDT
python-modernize-0.2-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/python-modernize-0.2-2.fc19
Comment 12 Fedora Update System 2013-07-31 03:50:39 EDT
python-modernize-0.2-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/python-modernize-0.2-2.fc18
Comment 13 Fedora Update System 2013-08-01 23:28:42 EDT
Package python-modernize-0.2-2.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing python-modernize-0.2-2.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-13959/python-modernize-0.2-2.fc18
then log in and leave karma (feedback).
Comment 14 Fedora Update System 2013-08-20 20:09:47 EDT
python-modernize-0.2-2.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Fedora Update System 2013-08-20 20:12:12 EDT
python-modernize-0.2-2.fc19 has been pushed to the Fedora 19 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.