Spec URL: http://mcepl.fedorapeople.org/tmp/python-flake8.spec SRPM URL: http://kojipkgs.fedoraproject.org//work/tasks/1762/4231762/python-flake8-1.4-1.fc18.src.rpm Description: Flake8 is a wrapper around these tools: - PyFlakes - pep8 - Ned's McCabe script Flake8 runs all tools by launching the single 'flake8' script, but ignores pep8 and PyFlakes extended options and just uses defaults. It displays the warnings in a per-file, merged output. It also adds a few features: - files that contains with this header are skipped:: # flake8: noqa - lines that contains a "# NOQA" comment at the end will not issue a warning. - a Mercurial hook. - a McCabe complexity checker. Fedora Account System Username: mcepl Build in http://koji.fedoraproject.org/koji/taskinfo?taskID=4231761
Package Review ============== Key: - = N/A x = Pass ! = Fail ? = Not evaluated ==== Generic ==== [x]: EXTRA Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: EXTRA Spec file according to URL is the same as in SRPM. [!]: MUST Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: MUST Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: MUST %build honors applicable compiler flags or justifies otherwise. [x]: MUST All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: MUST Buildroot is not present Note: Unless packager wants to package for EPEL5 this is fine [x]: MUST Package contains no bundled libraries. [x]: MUST Changelog in prescribed format. [x]: MUST Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) Note: Clean would be needed if support for EPEL is required [x]: MUST Sources contain only permissible code or content. [x]: MUST Each %files section contains %defattr if rpm < 4.4 Note: Note: defattr macros not found. They would be needed for EPEL5 [x]: MUST Macros in Summary, %description expandable at SRPM build time. [-]: MUST Package contains desktop file if it is a GUI application. [-]: MUST Development files must be in a -devel package [ ]: MUST Package requires other packages for directories it uses. [x]: MUST Package uses nothing in %doc for runtime. [x]: MUST Package is not known to require ExcludeArch. [x]: MUST Permissions on files are set properly. [x]: MUST Package does not contain duplicates in %files. [x]: MUST Package complies to the Packaging Guidelines [x]: MUST Spec file lacks Packager, Vendor, PreReq tags. [x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf would be needed if support for EPEL5 is required [-]: MUST Large documentation files are in a -doc subpackage, if required. [-]: MUST If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [!]: MUST License field in the package spec file matches the actual license. [x]: MUST Package consistently uses macro is (instead of hard-coded directory names). [x]: MUST Package is named using only allowed ascii characters. [x]: MUST Package is named according to the Package Naming Guidelines. [x]: MUST Package does not generate any conflict. [x]: MUST Package obeys FHS, except libexecdir and /usr/target. [!]: MUST Package must own all directories that it creates. [x]: MUST Package does not own files or directories owned by other packages. [x]: MUST Package installs properly. [x]: MUST Package is not relocatable. [x]: MUST Requires correct, justified where necessary. [!]: MUST Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: MUST Sources used to build the package match the upstream source, as provided in the spec URL. [x]: MUST Spec file is legible and written in American English. [x]: MUST Spec file name must match the spec package %{name}, in the format %{name}.spec. [-]: MUST Package contains systemd file(s) if in need. [x]: MUST File names are valid UTF-8. [x]: SHOULD Reviewer should test that the package builds in mock. [-]: SHOULD If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: SHOULD Dist tag is present. [x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q --requires). [x]: SHOULD Package functions as described. [x]: SHOULD Latest version is packaged. [x]: SHOULD Package does not include license text files separate from upstream. [x]: SHOULD SourceX / PatchY prefixed with %{name}. [x]: SHOULD SourceX is a working URL. [-]: SHOULD Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: SHOULD Package should compile and build into binary rpms on all supported architectures. [!]: SHOULD %check is present and all tests pass. [x]: SHOULD Packages should try to preserve timestamps of original installed files. [x]: SHOULD Spec use %global instead of %define. Rpmlint ------- Checking: python-flake8-1.4-1.fc16.noarch.rpm python-flake8-1.4-1.fc16.src.rpm python-flake8.noarch: W: spelling-error Summary(en_US) pyflakes -> flakes python-flake8.noarch: W: summary-not-capitalized C code checking using pep8 and pyflakes python-flake8.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/flake8/pep8.py 0644L /usr/bin/env python-flake8.noarch: W: no-manual-page-for-binary flake8 python-flake8.src: W: spelling-error Summary(en_US) pyflakes -> flakes python-flake8.src: W: summary-not-capitalized C code checking using pep8 and pyflakes 2 packages and 0 specfiles checked; 1 errors, 5 warnings. So to conclude: - License wise, the package looks fine but some file (ie: message.py) seems to come from another project/source. Could you just make sure there were also MIT ? - rpmlint generates some error/warnings that could be fixed (at least the summary and the shebang ones) - In the %files section, why not using %{python_sitelib}/%{modname}* directly ? - Have you look at running the unit-tests provided during the build ? Otherwise looks good, fix the rpmlint output and check for the test and the license question and I will approve it.
ping ?
Sorry, I missed your reply ... will take a look at it asap.
(In reply to comment #1) > - License wise, the package looks fine but some file (ie: message.py) seems > to come from another project/source. Could you just make sure there were > also MIT ? https://bitbucket.org/tarek/flake8/issue/26/unknown-source-of-some-files > So to conclude: > - rpmlint generates some error/warnings that could be fixed (at least the > summary and the shebang ones) > - Have you look at running the unit-tests provided during the build ? Upgraded .spec file: http://koji.fedoraproject.org/koji/taskinfo?taskID=4466971 http://kojipkgs.fedoraproject.org//work/tasks/6972/4466972/python-flake8-1.4-2.fc19.src.rpm
Sorry for the (very) low reaction time. I see upstream fixed the licensing issue, but your koji build has timed-out. Could you please re-upload the spec file? Thanks
Created attachment 667881 [details] new version of the SPEC file
Should we move-on with this one now that the dependency has been approved?
Oh yes please, I could use it too. Is there anything missing e.g. for review or else?
(In reply to comment #7) > Should we move-on with this one now that the dependency has been approved? please, go ahead and finish the review.
Could you please provide the srpm and the spec in an accessible place? (fedorapeople?)
(In reply to comment #10) > Could you please provide the srpm and the spec in an accessible place? > (fedorapeople?) http://mcepl.fedorapeople.org/tmp/python-flake8-2.0-1.el7.src.rpm http://mcepl.fedorapeople.org/tmp/python-flake8.spec
The srpm fails to build on my F18, apparently the README is either not there anymore or has been renamed
Shouldn't the BuildRequires contain python-setuptools as well?
any news here? Is there something I can do to help here?
(In reply to Matthias Runge from comment #14) > any news here? Is there something I can do to help here? Sorry, there is now http://mcepl.fedorapeople.org/tmp/python-flake8-2.0-2.el7.src.rpm and http://mcepl.fedorapeople.org/tmp/python-flake8.spec
Fails to build on a F19 mock due to missing python3-pyflakes
Corresponding logs in rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=5898306
Once that python3-pyflakes is in f20, IMHO we can continue here.
If you could also get this package into EL6 it would be much appreciated. Thanks!
OK, I took the freedom to fix the last minimal issues: * renamed python3 flake8 * also running nosetests for python3 SRPM: http://matthias-runge.de/fedora/python-flake8-2.0-3.fc20.src.rpm SPEC: http://matthias-runge.de/fedora/python-flake8.spec koji scratchbuild is here: http://koji.fedoraproject.org/koji/taskinfo?taskID=6139591
Hi Matthias, Thanks for the update, does this mean you are taking over this package from Matej? Have you discussed this with him? If so, you should open a new review and close this on as duplicate. If not done, I'll finish the review next week.
Hi, My intention was just to bring this a little step further; recently, I reached out for Matej in private to discuss, how to continue here.
(In reply to Matthias Runge from comment #22) > Hi, > > My intention was just to bring this a little step further; recently, I > reached out for Matej in private to discuss, how to continue here. I went through the package and I don't see the reason why this shouldn't be reviewed. I hope I'll be more following the development here. I am sorry. Please, go ahead with the review. I have made a koji scratch build http://koji.fedoraproject.org/koji/taskinfo?taskID=6146575 and my SRPM (but it is the same as in comment 20) is http://mcepl.fedorapeople.org/tmp/python-flake8-2.0-3.fc20.src.rpm SPEC file is http://mcepl.fedorapeople.org/tmp/python-flake8.spec
Ok, sorry for the late reply. The spec looks nice, it builds and license has been cleared. The last item I see is regarding the use the of the %{__python} macro which has been deprecated in favor of %{__python2}: https://fedoraproject.org/wiki/Packaging:Python#Macros Maybe the CONTRIBUTORS.txt could also be present in the python3 package. If you have a minute to upload a new version I will approve the review :)
Thank you for the review! Changed as proposed. Updated SRPM: http://www.matthias-runge.de/fedora/python-flake8-2.0-4.fc20.src.rpm SPEC: http://www.matthias-runge.de/fedora/python-flake8.spec
Ok, this package is APPROVED!
Thank you very much! New Package SCM Request ======================= Package Name: python-flake8 Short Description: code checking using pep8 and pyflakes Owners: mrunge mcepl Branches: f19 f20 el6
damn, mid-air collision ;)
Git done (by process-git-requests).
(In reply to Matěj Cepl from comment #28) > damn, mid-air collision ;) Did you want anything changed/else?
(In reply to Matthias Runge from comment #30) > (In reply to Matěj Cepl from comment #28) > > damn, mid-air collision ;) > > Did you want anything changed/else? no, more or less the same ... not sure whether you wanted to be a comaintainer.
python-flake8-2.0-4.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/python-flake8-2.0-4.fc20
python-flake8-2.0-4.fc20 has been pushed to the Fedora 20 testing repository.
python-flake8-2.0-4.fc20 has been pushed to the Fedora 20 stable repository.