Spec URL: http://corsepiu.fedorapeople.org/packages/gumbo-parser.spec SRPM URL: http://corsepiu.fedorapeople.org/packages/gumbo-parser-1.0-1.20130816git88ee911.fc20.src.rpm Description: Gumbo is an implementation of the HTML5 parsing algorithm implemented as a pure C99 library with no outside dependencies. It's designed to serve as a building block for other tools and libraries such as linters, validators, templating languages, and refactoring and analysis tools. Fedora Account System Username: corsepiu
Created attachment 812353 [details] review.txt Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30 Command line :/usr/bin/fedora-review -b 997780 Buildroot used: fedora-19-x86_64 Active plugins: Python, Generic, Shell-api, C/C++ Disabled plugins: Java, SugarActivity, Perl, R, PHP, Ruby Disabled flags: EPEL5, EXARCH, DISTTAG
Kohi scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6061098 MUST [!]: Package meets the Packaging Guidelines::Python From recent Guidelines change: Package don't contains BR: python2-devel or python3-devel The unversioned macro, %{__python} is deprecated. use {__python2} %{python2_sitelib} macros to be consistent Notice, I will prefer the python BR in python sub-package. [!]: Package is named according to the Package Naming Guidelines. Version 1.0 is not released, so this is a pre-release => Release: 0.1.... [!]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [!]: If (and only if) the source package includes the text of the license(s) "Common licenses that require including their texts with all derivative works include ASL 2.0..." => COPYING is now present on github SHOULD [!]: Package consistently uses macros (instead of hard-coded directory names). => Could use %{name} in URL, python description, ... COULD [!]: Too large wildcard, because I dislike them ;) %{_libdir}/libgumbo.so.1* %{python_sitelib}/gumbo*
Can you please check why _GumboNode.3 instead of GumboNode.3 ? (exactly why I dislike wide wildcard)
(In reply to Remi Collet from comment #2) > Kohi scratch build: > http://koji.fedoraproject.org/koji/taskinfo?taskID=6061098 > > MUST > > [!]: Package meets the Packaging Guidelines::Python > From recent Guidelines change: > Package don't contains BR: python2-devel or python3-devel > The unversioned macro, %{__python} is deprecated. > use {__python2} %{python2_sitelib} macros to be consistent Well, as you know, I consider this FPC decision to be a non-helpful mistake, exactly because of cases like these. - The python bindings of this package are optional. - The package's python bindings are not tied to any particular version of python. - The package is supposed to be build against the distribution's "default python". => Enforcing to use python2|3 means unnecessarily tying the package against a specific version of python. That said, thanks to the churn this all causes, I am quite strongly leaning towards not packaging the python bindings at all. For now, I have decided to unconditionally switch to python3, because I do not see much sense in adding support for an discontinued version of python in a new package. > Notice, I will prefer the python BR in python sub-package. I don't understand. > [!]: Package is named according to the Package Naming Guidelines. > Version 1.0 is not released, so this is a pre-release > => Release: 0.1.... Well, upstream is quite inconsistent on this. Internally, they consistently use version 1.0. In README.md, they are talking about 0.9. As the Release-tag is not of much importance (and doesn't make any difference to users) I an switching to using Release: 0.x.y > [!]: If the source package does not include license text(s) as a separate > file > from upstream, the packager SHOULD query upstream to include it. > [!]: If (and only if) the source package includes the text of the license(s) > > "Common licenses that require including their texts with all > derivative works include ASL 2.0..." > > => COPYING is now present on github Yes. Wasn't present at the time, I pulled the tarball. > SHOULD > > [!]: Package consistently uses macros (instead of hard-coded directory > names). > => Could use %{name} in URL, python description, ... Done. > COULD > > [!]: Too large wildcard, because I dislike them ;) > %{_libdir}/libgumbo.so.1* > %{python_sitelib}/gumbo* My personal preference differs. (In reply to Remi Collet from comment #3) > Can you please check why _GumboNode.3 instead of GumboNode.3 ? Upstream bug. They fixed it. Updated package with a couple of more issues fixed: Spec URL: http://corsepiu.fedorapeople.org/packages/gumbo-parser.spec SRPM URL: http://corsepiu.fedorapeople.org/packages/gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc21.src.rpm
I mostly agree on those python macro... but "we" have approve this ;) >> Notice, I will prefer the python BR in python sub-package. >I don't understand. I just mean to move the "BuildRequires: python3-setuptools python3-devel" in the "%package python" section of the spec. But probably it only make sense to me ;) [x]: Package meets the Packaging Guidelines::Python [x]: Package is named according to the Package Naming Guidelines. [x]: license text(s) [x]: Package consistently uses macros (instead of hard-coded directory names). So: APPROVED
New Package SCM Request ======================= Package Name: gumbo-parser Short Description: A HTML5 parser library Owners: corsepiu Branches: f18 f19 f20
First of all thanks for the review! (In reply to Remi Collet from comment #5) > I mostly agree on those python macro... but "we" have approve this ;) Likely we are going to see if/else %fedora/%rhel cascades in rpm-specs to switch to the different "default python" because %__python was banned. IMNSHO, this is stupid and needs to be revisited. > >> Notice, I will prefer the python BR in python sub-package. > >I don't understand. > > I just mean to move the "BuildRequires: python3-setuptools python3-devel" in > the "%package python" section of the spec. But probably it only make sense > to me ;) :=) AFAICT, it technically doesn't matter all.
Git done (by process-git-requests).
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc19
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc18
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc20 has been submitted as an update for Fedora 20. https://admin.fedoraproject.org/updates/gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc20
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc20 has been pushed to the Fedora 20 testing repository.
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc19 has been pushed to the Fedora 19 stable repository.
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc18 has been pushed to the Fedora 18 stable repository.
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc20 has been pushed to the Fedora 20 stable repository.