[Bug 997780] New: Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Bug ID: 997780
           Summary: Review Request: gumbo-parser - A HTML5 parser library
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail],
                    [e-mail]

Spec URL: http://corsepiu.fedorapeople.org/packages/gumbo-parser.spec
SRPM URL:
http://corsepiu.fedorapeople.org/packages/gumbo-parser-1.0-1.20130816git88ee...
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

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Remi Collet  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Remi Collet  ---
Created attachment 812353
  --> https://bugzilla.redhat.com/attachment.cgi?id=812353&action=edit
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

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Remi Collet  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|[e-mail]    |[e-mail]

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #2 from Remi Collet  ---
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*

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #3 from Remi Collet  ---
Can you please check why _GumboNode.3 instead of GumboNode.3 ?
(exactly why I dislike wide wildcard)

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #4 from Ralf Corsepius  ---
(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.20131001gitd9...

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Remi Collet  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review+

--- Comment #5 from Remi Collet  ---
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

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Ralf Corsepius  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-cvs?

--- Comment #6 from Ralf Corsepius  ---
New Package SCM Request
=======================
Package Name: gumbo-parser
Short Description: A HTML5 parser library
Owners: corsepiu
Branches: f18 f19 f20

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #7 from Ralf Corsepius  ---
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.

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Jon Ciesla  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-cvs?                 |fedora-cvs+

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #8 from Jon Ciesla  ---
Git done (by process-git-requests).

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |MODIFIED

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #9 from Fedora Update System  ---
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.20131001gitd90e...

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #10 from Fedora Update System  ---
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.20131001gitd90e...

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #11 from Fedora Update System  ---
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.20131001gitd90e...

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #12 from Fedora Update System  ---
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc20 has been pushed to the Fedora 20
testing repository.

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
   Fixed In Version|                            |gumbo-parser-1.0-0.2.201310
                   |                            |01gitd90ea2b.fc19
         Resolution|---                         |ERRATA
        Last Closed|                            |2013-10-26 23:56:09

--- Comment #13 from Fedora Update System  ---
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc19 has been pushed to the Fedora 19
stable repository.

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

--- Comment #14 from Fedora Update System  ---
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc19 has been pushed to the Fedora 19
stable repository.

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Fixed In Version|gumbo-parser-1.0-0.2.201310 |gumbo-parser-1.0-0.2.201310
                   |01gitd90ea2b.fc19           |01gitd90ea2b.fc18

--- Comment #15 from Fedora Update System  ---
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc18 has been pushed to the Fedora 18
stable repository.

[Bug 997780] Review Request: gumbo-parser - A HTML5 parser library

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=997780

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
   Fixed In Version|gumbo-parser-1.0-0.2.201310 |gumbo-parser-1.0-0.2.201310
                   |01gitd90ea2b.fc18           |01gitd90ea2b.fc20

--- Comment #16 from Fedora Update System  ---
gumbo-parser-1.0-0.2.20131001gitd90ea2b.fc20 has been pushed to the Fedora 20
stable repository.