Bug 958007

Summary: Review Request: python-hacking - OpenStack Hacking Guideline Enforcement
Product: [Fedora] Fedora Reporter: Matthias Runge <mrunge>
Component: Package ReviewAssignee: Ankur Sinha (FranciscoD) <sanjay.ankur>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: i, package-review, sanjay.ankur
Target Milestone: ---Flags: sanjay.ankur: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: python-hacking-0.8.0-1.fc20 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-12-14 03:34:27 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: 839056    
Bug Blocks:    

Description Matthias Runge 2013-04-30 06:10:02 UTC
Spec URL: http://www.matthias-runge.de/fedora/python-hacking.spec
SRPM URL: http://www.matthias-runge.de/fedora/python-hacking-0.5.3-1.fc18.src.rpm
Description: OpenStack Hacking Guidline Enforcement
Fedora Account System Username: mrunge

BR python-flake8 is https://bugzilla.redhat.com/show_bug.cgi?id=839056

Comment 1 Ankur Sinha (FranciscoD) 2013-05-03 10:53:21 UTC
I'll review this. Doing the non building bits for the time being. Will check the building bits once the BR is approved.

Comment 2 Matthias Runge 2013-06-07 10:51:12 UTC
Updated to use also checks as well:
SPEC: http://www.matthias-runge.de/fedora/python-hacking.spec
SRPM: http://www.matthias-runge.de/fedora/python-hacking-0.5.3-2.fc19.src.rpm

Comment 3 Matthias Runge 2013-09-23 08:37:23 UTC
updated to version 0.7.2
SPEC: http://www.matthias-runge.de/fedora/python-hacking.spec
SRPM: http://www.matthias-runge.de/fedora/python-hacking-0.7.2-1.fc19.src.rpm
sadly, still no progress in python-flake8

Comment 4 Matthias Runge 2013-11-19 09:58:00 UTC
now that python-flake8 is in the repos:

SRPM: http://www.matthias-runge.de/fedora/python-hacking-0.8.0-1.fc20.src.rpm
SPEC: http://www.matthias-runge.de/fedora/python-hacking.spec

Comment 5 Christopher Meng 2013-11-19 10:00:56 UTC
Package summary should be(found on github page, because I don't know guidline is what so I just visited it):

OpenStack Hacking Style Checks utility.

Also please change the bug summary.

Comment 6 Matthias Runge 2013-11-19 10:06:47 UTC
(In reply to Christopher Meng from comment #5)
> Package summary should be(found on github page, because I don't know
> guidline is what so I just visited it):
> 
> OpenStack Hacking Style Checks utility.
> 
> Also please change the bug summary.

Thank you for your suggestion. I will stick to the text from pypi. AFAIK there is no rule for enforcing one or the other summary.

https://pypi.python.org/pypi/hacking
First line:
OpenStack Hacking Guidline Enforcement

Esp. that's the summary from
https://github.com/openstack-dev/hacking/blob/master/setup.cfg

Thanks.

Comment 7 Ankur Sinha (FranciscoD) 2013-11-19 10:12:54 UTC
(In reply to Matthias Runge from comment #4)
> now that python-flake8 is in the repos:
> 
> SRPM: http://www.matthias-runge.de/fedora/python-hacking-0.8.0-1.fc20.src.rpm
> SPEC: http://www.matthias-runge.de/fedora/python-hacking.spec

Ah! Great. Finally :D

I'll review it ASAP Matthias. Thanks for the update.

Oh, and I think the summary is fine. The maintainer can decide the best summary if there are many of them. Although, you might want to correct the spelling of "guideline". I'm sure rpmlint will report it anyway. ;)

Review coming up soon.

Thanks,
Warm regards,
Ankur

Comment 8 Ankur Sinha (FranciscoD) 2013-11-22 11:42:25 UTC
Here's the review:

[+] OK
[-] NA
[?] Issue

** Mandatory review guidelines: **
 [+] rpmlint output:
[asinha@ankur-laptop  SRPMS]$ rpmlint ../SPECS/python-hacking.spec ./python-hacking-0.8.0-1.fc20.src.rpm /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
3 packages and 1 specfiles checked; 0 errors, 0 warnings.
[asinha@ankur-laptop  SRPMS]$
 [+] License is acceptable (...)
[asinha@ankur-laptop  hacking-0.8.0]$ find . -name "*" -exec licensecheck '{}' \; | sed '/UNKNOWN/d'
./setup.py: Apache (v2.0) GENERATED FILE
./setup.py: Apache (v2.0) GENERATED FILE
./hacking/config.py: *No copyright* Apache (v2.0)
./hacking/core.py: Apache (v2.0)
./hacking/__init__.py: Apache (v2.0)
./hacking/config.py: *No copyright* Apache (v2.0)
./hacking/tests/test_doctest.py: Apache (v2.0)
./hacking/tests/test_local.py: Apache (v2.0)
./hacking/tests/test_config.py: *No copyright* Apache (v2.0)
./hacking/tests/test_import_exceptions.py: *No copyright* Apache (v2.0)
./hacking/tests/__init__.py: Apache (v2.0)
./hacking/tests/test_gittest.py: Apache (v2.0)
./hacking/tests/test_doctest.py: Apache (v2.0)
./hacking/tests/test_local.py: Apache (v2.0)
./hacking/tests/test_config.py: *No copyright* Apache (v2.0)
./hacking/tests/test_import_exceptions.py: *No copyright* Apache (v2.0)
./hacking/tests/__init__.py: Apache (v2.0)
./hacking/tests/test_gittest.py: Apache (v2.0)
./hacking/core.py: Apache (v2.0)
./hacking/__init__.py: Apache (v2.0)

 [+] License field in spec is correct
 [+] License files included in package %docs if included in source package
 [+] License files installed when any subpackage combination is installed
 [+] Spec written in American English
 [+] Spec is legible
 [+] Sources match upstream unless altered to fix permissibility issues
[asinha@ankur-laptop  SPECS(master *=)]$ review-md5check.sh python-hacking.spec
Getting http://pypi.python.org/packages/source/h/hacking/hacking-0.8.0.tar.gz to /tmp/review/hacking-0.8.0.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  120k  100  120k    0     0  62641      0  0:00:01  0:00:01 --:--:-- 82236
26c9d04e894e45ea5e0e68fe710fd5d3  /tmp/review/hacking-0.8.0.tar.gz
26c9d04e894e45ea5e0e68fe710fd5d3  /home/asinha/rpmbuild/SOURCES/hacking-0.8.0.tar.gz
removed ‘/tmp/review/hacking-0.8.0.tar.gz’
removed directory: ‘/tmp/review’
[asinha@ankur-laptop  SPECS(master *=)]$

 [+] Build succeeds on at least one primary arch
 [-] Build succeeds on all primary arches or has ExcludeArch + bugs filed
 [+] BuildRequires correct, justified where necessary
 [+] Locales handled with %find_lang, not %_datadir/locale/*
 [-] %post, %postun call ldconfig if package contains shared .so files
 [+] No bundled libs
 [-] Relocatability is justified
 [+] Package owns all directories it creates
[asinha@ankur-laptop  result]$ rpmls ./python-hacking-0.8.0-1.fc21.noarch.rpm
drwxr-xr-x  /usr/lib/python2.7/site-packages/hacking
drwxr-xr-x  /usr/lib/python2.7/site-packages/hacking-0.8.0-py2.7.egg-info
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking-0.8.0-py2.7.egg-info/PKG-INFO
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking-0.8.0-py2.7.egg-info/SOURCES.txt
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking-0.8.0-py2.7.egg-info/dependency_links.txt
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking-0.8.0-py2.7.egg-info/entry_points.txt
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking-0.8.0-py2.7.egg-info/not-zip-safe
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking-0.8.0-py2.7.egg-info/top_level.txt
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/__init__.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/__init__.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/__init__.pyo
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/config.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/config.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/config.pyo
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/core.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/core.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/core.pyo
drwxr-xr-x  /usr/lib/python2.7/site-packages/hacking/tests
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/__init__.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/__init__.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/__init__.pyo
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_config.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_config.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_config.pyo
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_doctest.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_doctest.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_doctest.pyo
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_gittest.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_gittest.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_gittest.pyo
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_import_exceptions.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_import_exceptions.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_import_exceptions.pyo
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_local.py
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_local.pyc
-rw-r--r--  /usr/lib/python2.7/site-packages/hacking/tests/test_local.pyo
drwxr-xr-x  /usr/share/doc/python-hacking
-rw-r--r--  /usr/share/doc/python-hacking/LICENSE
-rw-r--r--  /usr/share/doc/python-hacking/README.rst
drwxr-xr-x  /usr/share/doc/python-hacking/html
drwxr-xr-x  /usr/share/doc/python-hacking/html/_sources
-rw-r--r--  /usr/share/doc/python-hacking/html/_sources/index.txt
drwxr-xr-x  /usr/share/doc/python-hacking/html/_static
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/ajax-loader.gif
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/basic.css
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/comment-bright.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/comment-close.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/comment.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/default.css
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/doctools.js
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/down-pressed.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/down.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/file.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/header-line.gif
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/header_bg.jpg
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/jquery.js
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/jquery.tweet.js
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/minus.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/nature.css
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/openstack_logo.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/plus.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/pygments.css
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/searchtools.js
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/tweaks.css
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/underscore.js
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/up-pressed.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/up.png
-rw-r--r--  /usr/share/doc/python-hacking/html/_static/websupport.js
-rw-r--r--  /usr/share/doc/python-hacking/html/genindex.html
-rw-r--r--  /usr/share/doc/python-hacking/html/index.html
-rw-r--r--  /usr/share/doc/python-hacking/html/objects.inv
-rw-r--r--  /usr/share/doc/python-hacking/html/search.html
-rw-r--r--  /usr/share/doc/python-hacking/html/searchindex.js
[asinha@ankur-laptop  result]$

 [-] Package requires others for directories it uses but does not own
 [+] No duplication in %files unless necessary for license files
 [+] File permissions are sane
 [+] Package contains permissible code or content
 [-] Large docs go in -doc subpackage
HTML doc can be put in another subpackage if the maintainer wishes.

 [+] %doc files not required at runtime
 [-] Static libs go in -static package/virtual Provides
 [-] Development files go in -devel package
 [-] -devel packages Require base with fully-versioned dependency, %_isa
 [+] No .la files
 [-] GUI app uses .desktop file, installs it with desktop-file-install
 [+] File list does not conflict with other packages' without justification
 [+] File names are valid UTF-8

** Optional review guidelines: **
 [-] Query upstream about including license files
 [-] Translations of description, summary
 [+] Builds in mock
 [-] Builds on all arches
 [-] Functions as described (e.g. no crashes)
^ 
Not checked. Please do check.

 [-] Scriptlets are sane
 [-] Subpackages require base with fully-versioned dependency if sensible
 [-] .pc file subpackage placement is sensible
 [+] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
 [-] Include man pages if available

Naming guidelines:
 [+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+
 [+] Package names are sane
 [+] No naming conflicts
 [+] Spec file name matches base package name
 [+] Version is sane
 [+] Version does not contain ~
 [+] Release is sane
 [+] %dist tag
 [-] Case used only when necessary
 [-] Renaming handled correctly

Packaging guidelines:
 [+] Useful without external bits
 [+] No kmods
 [+] Pre-built binaries, libs removed in %prep
^ Egg info removed

 [+] Sources contain only redistributable code or content
 [+] Spec format is sane
 [+] Package obeys FHS, except libexecdir, /run, /usr/target
 [+] No files in /bin, /sbin, /lib* on >= F17
 [+] Programs run before FS mounting use /run instead of /var/run
 [+] Binaries in /bin, /sbin do not depend on files in /usr on < F17
 [+] No files under /srv, /opt, /usr/local
 [+] Changelog in prescribed format
 [+] No Packager, Vendor, Copyright, PreReq tags
 [+] Summary does not end in a period
 [-] Correct BuildRoot tag on < EL6
 [-] Correct %clean section on < EL6
^
Please update the spec if you intend to keep the package for EL6

 [?] Requires correct, justified where necessary
^
Please comment the explicit requires before you commit to SCM. Not a blocker, but it'll enable other packagers to better understand why these are required.

 [+] Summary, description do not use trademarks incorrectly
 [+] All relevant documentation is packaged, appropriately marked with %doc
 [+] Doc files do not drag in extra dependencies (e.g. due to +x)
 [-] Code compilable with gcc is compiled with gcc
 [-] Build honors applicable compiler flags or justifies otherwise
 [-] PIE used for long-running/root daemons, setuid/filecap programs
 [+] Useful -debuginfo package or disabled and justified
 [-] Package with .pc files Requires pkgconfig on < EL6
 [+] No static executables
 [+] Rpath absent or only used for internal libs
 [-] Config files marked with %config(noreplace) or justified %config
 [-] No config files under /usr
 [-] Third party package manager configs acceptable, in %_docdir
 [-] .desktop files are sane
 [+] Spec uses macros consistently
 [+] Spec uses macros instead of hard-coded names where appropriate
 [+] Spec uses macros for executables only when configurability is needed
 [-] %makeinstall used only when alternatives don't work
 [-] Macros in Summary, description are expandable at srpm build time
 [-] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
 [-] No software collections (scl)
 [-] Macro files named /etc/rpm/macros.%name
 [-] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
 [+] %global, not %define
 [-] Package translating with gettext BuildRequires it
 [-] Package translating with Linguist BuildRequires qt-devel
 [-] File ops preserve timestamps
 [-] Parallel make
 [-] No Requires(pre,post) notation
 [-] User, group creation handled correctly (See Packaging:UsersAndGroups)
 [-] Web apps go in /usr/share/%name, not /var/www
 [-] Conflicts are justified
 [+] One project per package
 [+] No bundled fonts
 [-] Patches have appropriate commentary
 [+] Available test suites executed in %check
 [-] tmpfiles.d used for /run, /run/lock on >= F15

 ** Python guidelines: **
 [+] Runtime Requires correct
^
Please comment the requires

 [-] Python macros declared on < EL6
^
Please modify the spec if you intend to maintain the package for EL6

 [+] All .py files packaged with .pyc, .pyo counterparts
 [+] Includes .egg-info files/directories when generated
 [+] Provides/Requires properly filtered
[asinha@ankur-laptop  result]$ review-req-check
== python-hacking-0.8.0-1.fc21.noarch.rpm ==
Provides:
python-hacking = 0.8.0-1.fc21

Requires:
pyflakes
python(abi) = 2.7
python-flake8
python-pbr
python-six

== python-hacking-0.8.0-1.fc21.src.rpm ==
Provides:

Requires:
python2-devel
python-setuptools
python-d2to1
python-pbr
python-sphinx
python-flake8
python-subunit
python-testrepository
python-testscenarios
python-testtools
python-oslo-sphinx
python-pep8
python-six
pyflakes
python-mccabe

[asinha@ankur-laptop  result]$

 [-] Code that invokes gtk.gdk.get_pixels_array() Requires numpy


Package looks good to me. Please do check if it's py3 compatible and modify the spec accordingly. 


XXXX APPROVED XXXX

Comment 9 Matthias Runge 2013-11-22 13:13:13 UTC
Thank you very much!

New Package SCM Request
=======================
Package Name: python-hacking
Short Description: OpenStack Hacking Guideline Enforcement
Owners: mrunge
Branches: f19 f20

Comment 10 Gwyn Ciesla 2013-11-22 13:15:57 UTC
Git done (by process-git-requests).

Comment 11 Fedora Update System 2013-11-22 18:08:50 UTC
python-hacking-0.8.0-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/python-hacking-0.8.0-1.fc20

Comment 12 Fedora Update System 2013-11-24 03:38:22 UTC
python-hacking-0.8.0-1.fc20 has been pushed to the Fedora 20 testing repository.

Comment 13 Fedora Update System 2013-12-14 03:34:27 UTC
python-hacking-0.8.0-1.fc20 has been pushed to the Fedora 20 stable repository.