Bug 997780
Summary: | Review Request: gumbo-parser - A HTML5 parser library | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ralf Corsepius <rc040203> | ||||
Component: | Package Review | Assignee: | Remi Collet <fedora> | ||||
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | fedora, notting, package-review | ||||
Target Milestone: | --- | Flags: | fedora:
fedora-review+
gwync: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc20 | Doc Type: | Bug Fix | ||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2013-10-27 03:56:09 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: | |||||||
Attachments: |
|
Description
Ralf Corsepius
2013-08-16 08:24:58 UTC
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.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. |