Bug 973084 - Review Request: htmlcleaner - HtmlCleaner is open-source HTML parser written in Java
Summary: Review Request: htmlcleaner - HtmlCleaner is open-source HTML parser written ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Björn Esser (besser82)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 894413
TreeView+ depends on / blocked
 
Reported: 2013-06-11 09:09 UTC by marcindulak
Modified: 2013-07-22 00:34 UTC (History)
5 users (show)

Fixed In Version: htmlcleaner-2.2.1-2.fc19
Clone Of:
Environment:
Last Closed: 2013-07-19 13:22:17 UTC
Type: ---
Embargoed:
besser82: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description marcindulak 2013-06-11 09:09:42 UTC
Spec URL: https://svn.fysik.dtu.dk/projects/rpmbuild/trunk/SPECS/htmlcleaner.spec
SRPM URL: http://download.opensuse.org/repositories/home:/marcindulak/Fedora_18/src/htmlcleaner-2.2.1-6.1.src.rpm
Description: HtmlCleaner is open-source HTML parser written in Java. HTML found on Web is usually dirty, ill-formed and unsuitable for further processing. For any serious consumption of such documents, it is necessary to first clean up the mess and bring the order to tags, attributes and ordinary text. For the given HTML document, HtmlCleaner reorders individual elements and produces well-formed XML. By default, it follows similar rules that the most of web browsers use in order to create Document Object Model. However, user may provide custom tag and rule set for tag filtering and balancing.
Fedora Account System Username:

Comment 1 Björn Esser (besser82) 2013-06-11 09:25:47 UTC
Sign-up on Fedora Account System, if you haven't done already and provide your FAS-username, please.

Added Blocks: FE-NEEDSPONSOR, since look-up for email-address in packager-group gave me no result.

Comment 2 marcindulak 2013-06-11 10:10:57 UTC
Fedora Account System Username: marcindulak

Comment 3 Björn Esser (besser82) 2013-06-11 10:44:49 UTC
Thanks!  Don't forget to sign "Contributor Agreement" inside your FAS-account and to request webspace for direct file-linking (you need to login with your FAS-credentials):

https://fedorahosted.org/packager-sponsors/newticket

Just select "initial package hosting request" and refer to this bug in your ticket.  Upload your spec and srpm to your fedorapeople webspace when it's ready and update the links, please.

Comment 4 marcindulak 2013-06-12 08:17:27 UTC
The https://fedorahosted.org/packager-sponsors/ticket/69 step
is blocking me for almost a day now

Comment 5 Michael Schwendt 2013-06-12 08:26:54 UTC
Please show a little bit of patience. It's a ticketing system with human volunteers working on those tasks.

Comment 6 marcindulak 2013-06-12 10:57:32 UTC
(In reply to Michael Schwendt from comment #5)
> Please show a little bit of patience. It's a ticketing system with human
> volunteers working on those tasks.

OK

Comment 8 Mikolaj Izdebski 2013-06-20 09:19:52 UTC
Disclaimer: I am not a sponsor of packager group so I can't do a formal review. I am adding comments as member of Java SIG and maintainer of Maven in Fedora.

1. You can use -f argument to %files macro. For example, instead of:
  %files
  %{_javadir}/%{name}.jar
  %{_mavendepmapfragdir}/%{name}
  %{_mavenpomdir}/JPP-%{name}.pom
you can simply write
  %files -f .mfiles

2. The call to %add_maven_depmap can be simplified, just remove both arguments.

3. "rm -rf $RPM_BUILD_ROOT" is not needed and should be removed.  (It would be needed for EPEL 5, but there is no Maven in EPEL.)

4. Group tags are not used in Fedora and can be removed.

5. Package descriptions should be wrapped up at 80 columns.  (Other lines can be longer, but should also be kept short if possible.)

6. Instead of using unzip you can use jar utility.  It is included with java-devel, so you wouldn't need to add BR on unzip.  Basically remove that BR and s/unzip/jar xf/ in %prep.

7. To patch pom.xml files you are recommended to use %pom_* macros. Instead of adding patch a single line in prep would suffice:
  %pom_remove_plugin :maven-gpg-plugin

Comment 9 Christopher Meng 2013-06-20 09:32:08 UTC
Besides these from Mikolaj Izdebski, you also need to:

Cleanup your spec, DO NOT mix tab and space.

I think Fedora 17 is near EOL, so you can ignore Fedora 18-.

Comment 10 Mikolaj Izdebski 2013-06-20 09:43:18 UTC
(In reply to Marcin.Dulak from comment #4)
> The https://fedorahosted.org/packager-sponsors/ticket/69 step
> is blocking me for almost a day now

You may have better chances finding a reviewer quickly if you block Java SIG tracker bug #652183 (FE-JAVASIG), post on java-devel mailing list or join #fedora-java on IRC and ask.

Comment 11 Björn Esser (besser82) 2013-06-20 12:57:57 UTC
Package has some issues. I'll collect things found by others in previous comments and those I found in my report. build.log shows nothing to worry.

#####

Package Review
==============

Legend:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Maven packages should use new style packaging
  Note: If possible update your package to latest guidelines
  See: https://fedoraproject.org/wiki/Packaging:Java#Apache_Maven

  ---> If you're really in the need having/using this in F18 you can keep
       "old-style" until F18 goes EOL, but you should add some comment about
       changing it to new style after F18 is EOL'ed.

       Otherwise you should change it to new-style immediatly and only build
       your package for F19+, only.

- All build dependencies are listed in BuildRequires, except for any that are
  listed in the exceptions section of Packaging Guidelines.
  Note: These BR are not needed: unzip
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

  ---> you can safely remove this BR, it's automaticly avail on all build-envs

- Packages have proper BuildRequires/Requires on jpackage-utils
  Note: Maven packages do not need to (Build)Require jpackage-utils. It is
  pulled in by maven-local
  See: https://fedoraproject.org/wiki/Packaging:Java

  ---> there's no need for BR: java-devel jpackage-utils,
       they are installed in build-env by dependency

       No need for conditional on BRs, too. In this case it would be needed
       for Fedora <= 17, only.  maven-local is avail on F18+

- Javadoc subpackages should not have Requires: jpackage-utils
  Note: jpackage-utils requires are automatically generated by the buildsystem
  See: https://fedoraproject.org/wiki/Packaging:Java

  ---> please remove Requires: jpackage-utils,
       it's picked-up by autorequires

- rpmlint complains (as seen below, too):
    htmlcleaner.spec:7: W: mixed-use-of-spaces-and-tabs (spaces: line 7, tab: line 1)

    ---> use tabs OR spaces for spacing columns, don't mix. see: comment #9

    htmlcleaner.noarch: W: name-repeated-in-summary C HtmlCleaner

    ---> proposed solution: `Summary:	HTML parser written in Java` should be fine

    htmlcleaner.noarch: E: description-line-too-long C HtmlCleaner is open-source HTML parser written in Java. ...

    ---> please split lines @80 char max.

    htmlcleaner.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/htmlcleaner-2.2.1/licence.txt

    ---> this can be easiely fixed:
         sed -i -e 's!\r!!g' licence.txt
         see: https://fedoraproject.org/wiki/Common_Rpmlint_issues#wrong-file-end-of-line-encoding

    The rest of complains is just repeated from the above.

    3 packages and 1 specfiles checked; 2 errors, 5 warnings.

===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     htmlcleaner-javadoc

     ---> false positive: docs-pkgs should not require the "binary" app.
          btw. noarch-pkgs mustn't have %{?_isa}-macro on requires.

[!]: Package complies to the Packaging Guidelines

     ---> issues are found

[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (3 clause)", "Unknown or generated". 13 files have unknown license.
     Detailed output of licensecheck in
     /home/bjoern.esser/fedora/review/973084-htmlcleaner/licensecheck.txt

     ---> License-Tag should be: "BSD with advertising", since license
          is 3-clause-BSD in fact.

[!]: License file installed when any subpackage combination is installed.

     ---> javadoc-pkg should include license-file, too. This is OK according
          to guidelines, see:
          https://fedoraproject.org/wiki/Packaging/Guidelines#Duplicate_Files
          https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing

[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[!]: Requires correct, justified where necessary.

     ---> Javadoc subpackages should not have Requires: jpackage-utils
          There's no need for explict requires on jpackage-utils, it is
          added by autorequires if neccesary

[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: 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.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).

Java:
[x]: Fully versioned dependency in subpackages, if present.
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
[x]: Bundled jar/class files should be removed before build

Maven:
[x]: Pom files have correct Maven mapping
     Note: Some add_maven_depmap calls found. Please check if they are correct
     or update to latest guidelines
[x]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[x]: Old add_to_maven_depmap macro is not being used
[x]: Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-
     utils for %update_maven_depmap macro
[x]: Package DOES NOT use %update_maven_depmap in %post/%postun
[x]: Packages use %{_mavenpomdir} instead of %{_datadir}/maven2/poms

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[!]: Final provides and requires are sane (see attachments).

     ---> javadoc subpackages should not have Requires: jpackage-utils

[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[!]: Patches link to upstream bugs/comments/lists or are otherwise justified.

     ---> instead of using a patch here, you should use %pom_* macros
          like proposed in comment #8

[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

Java:
[x]: Package uses upstream build method (ant/maven/etc.)
[x]: Packages are noarch unless they use JNI

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: htmlcleaner-2.2.1-1.fc20.noarch.rpm
          htmlcleaner-javadoc-2.2.1-1.fc20.noarch.rpm
htmlcleaner.noarch: W: name-repeated-in-summary C HtmlCleaner
htmlcleaner.noarch: E: description-line-too-long C HtmlCleaner is open-source HTML parser written in Java. HTML found on Web is usually dirty, ill-formed and unsuitable for further processing. For any serious consumption of such documents, it is necessary to first clean up the mess and bring the order to tags, attributes and ordinary text. For the given HTML document, HtmlCleaner reorders individual elements and produces well-formed XML. By default, it follows similar rules that the most of web browsers use in order to create Document Object Model. However, user may provide custom tag and rule set for tag filtering and balancing.
htmlcleaner.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/htmlcleaner-2.2.1/licence.txt
2 packages and 0 specfiles checked; 1 errors, 2 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint htmlcleaner htmlcleaner-javadoc
htmlcleaner.noarch: W: name-repeated-in-summary C HtmlCleaner
htmlcleaner.noarch: E: description-line-too-long C HtmlCleaner is open-source HTML parser written in Java. HTML found on Web is usually dirty, ill-formed and unsuitable for further processing. For any serious consumption of such documents, it is necessary to first clean up the mess and bring the order to tags, attributes and ordinary text. For the given HTML document, HtmlCleaner reorders individual elements and produces well-formed XML. By default, it follows similar rules that the most of web browsers use in order to create Document Object Model. However, user may provide custom tag and rule set for tag filtering and balancing.
htmlcleaner.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/htmlcleaner-2.2.1/licence.txt
2 packages and 0 specfiles checked; 1 errors, 2 warnings.
# echo 'rpmlint-done:'



Requires
--------
htmlcleaner (rpmlib, GLIBC filtered):
    java
    jpackage-utils

htmlcleaner-javadoc (rpmlib, GLIBC filtered):
    jpackage-utils



Provides
--------
htmlcleaner:
    htmlcleaner
    mvn(net.sourceforge.htmlcleaner:htmlcleaner)

htmlcleaner-javadoc:
    htmlcleaner-javadoc



Source checksums
----------------
http://downloads.sourceforge.net/project/htmlcleaner/htmlcleaner/htmlcleaner v2.2.1/htmlcleaner-2.2.1-src.zip :
  CHECKSUM(SHA256) this package     : d1f045efff57d266c94e6b87e6685c14d7fbec3aac648f6020cc69812fe0be31
  CHECKSUM(SHA256) upstream package : d1f045efff57d266c94e6b87e6685c14d7fbec3aac648f6020cc69812fe0be31


Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-rawhide-x86_64
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 973084

#####

please fix the issues, update changelog/revision in spec and I'll take another look on this.

If you want to convince someone to sponsor you, I recommend you should follow the proposals from the wiki [1] (esp. in doing `informal reviews`) and posting links to them here.

[1] http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#Convincing_someone_to_sponsor_you

Comment 12 marcindulak 2013-06-20 15:42:39 UTC
http://marcindulak.fedorapeople.org/packages/htmlcleaner/v02/htmlcleaner.spec
http://marcindulak.fedorapeople.org/packages/htmlcleaner/v02/htmlcleaner-2.2.1-2.fc20.src.rpm
Issues from bug #973084 comment #11 should be fixed.
By the way what's the correct way of running rpmlint, when i do:
rpmlint htmlcleaner.spec
i don't get any errors/warnings.

Comment 13 Mikolaj Izdebski 2013-06-20 15:50:15 UTC
(In reply to Marcin.Dulak from comment #12)
> By the way what's the correct way of running rpmlint, when i do:
> rpmlint htmlcleaner.spec
> i don't get any errors/warnings.

You should run rpmlint on SRPM instead of spec file.

Comment 14 Björn Esser (besser82) 2013-06-20 17:29:14 UTC
Package is fine now, except for License-Tag in spec:

's!BSD$!& with advertising!'

#####

Package Review
==============

Legend:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Fully versioned dependency in subpackages, if present.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     htmlcleaner-javadoc

     ---> false positive, see comment #11

[x]: Package complies to the Packaging Guidelines
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "BSD (3 clause)", "Unknown or generated". 13 files have unknown license.
     Detailed output of licensecheck in
     /home/bjoern.esser/fedora/review/973084-htmlcleaner/licensecheck.txt

     ---> see above: 's!BSD!& with advertising!'

[x]: License file installed when any subpackage combination is installed.
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[-]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: 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.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.

Java:
[x]: Packages have proper BuildRequires/Requires on jpackage-utils
     Note: Maven packages do not need to (Build)Require jpackage-utils. It is
     pulled in by maven-local
[x]: Fully versioned dependency in subpackages, if present.
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadoc subpackages should not have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
[x]: Bundled jar/class files should be removed before build

Maven:
[x]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[x]: Pom files have correct Maven mapping
[x]: Maven packages should use new style packaging
[x]: Old add_to_maven_depmap macro is not being used
[x]: Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-
     utils for %update_maven_depmap macro
[x]: Package DOES NOT use %update_maven_depmap in %post/%postun
[x]: Packages use %{_mavenpomdir} instead of %{_datadir}/maven2/poms

===== SHOULD items =====

Generic:
[-]: 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]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

Java:
[x]: Package uses upstream build method (ant/maven/etc.)
[x]: Packages are noarch unless they use JNI

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: htmlcleaner-2.2.1-2.fc20.noarch.rpm
          htmlcleaner-javadoc-2.2.1-2.fc20.noarch.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint htmlcleaner htmlcleaner-javadoc
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



Requires
--------
htmlcleaner (rpmlib, GLIBC filtered):
    java
    jpackage-utils
    mvn(org.jdom:jdom)

htmlcleaner-javadoc (rpmlib, GLIBC filtered):
    jpackage-utils



Provides
--------
htmlcleaner:
    htmlcleaner
    mvn(net.sourceforge.htmlcleaner:htmlcleaner)

htmlcleaner-javadoc:
    htmlcleaner-javadoc



Source checksums
----------------
http://downloads.sourceforge.net/project/htmlcleaner/htmlcleaner/htmlcleaner v2.2.1/htmlcleaner-2.2.1-src.zip :
  CHECKSUM(SHA256) this package     : d1f045efff57d266c94e6b87e6685c14d7fbec3aac648f6020cc69812fe0be31
  CHECKSUM(SHA256) upstream package : d1f045efff57d266c94e6b87e6685c14d7fbec3aac648f6020cc69812fe0be31


Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-rawhide-x86_64
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 973084

#####

Good work! Just fix the issue with license-tag (already mentioned in comment #11) and package will be pristine.

Comment 15 marcindulak 2013-06-20 19:59:53 UTC
(In reply to Björn Esser from comment #14)
> Package is fine now, except for License-Tag in spec:
> 
> 's!BSD$!& with advertising!'

the /usr/share/doc/htmlcleaner-2.2.1/licence.txt file
looks to me closer to
https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#3ClauseBSD
The modification are:
- range of <YEAR-YEAR> instead of <YEAR>
- a disclaimer at the bottom of the file saying:
  "You can contact Vladimir Nikic by sending e-mail to
  nikic_vladimir. Please include the word "HtmlCleaner" in the
  subject line."
It's right that 13 files do not contain license header, but why
changing the license to "BSD with advertising"?

> #####
> 
> Package Review
> ==============
> 
> Legend:
> [x] = Pass
> [!] = Fail
> [-] = Not applicable
> [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> 
> ===== MUST items =====
> 
> Generic:
> [x]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.
> [x]: Package contains no bundled libraries without FPC exception.
> [x]: Changelog in prescribed format.
> [x]: Sources contain only permissible code or content.
> [-]: Package contains desktop file if it is a GUI application.
> [x]: Development files must be in a -devel package
> [x]: Package requires other packages for directories it uses.
> [x]: Package uses nothing in %doc for runtime.
> [x]: Package is not known to require ExcludeArch.
> [x]: Fully versioned dependency in subpackages, if present.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
>      htmlcleaner-javadoc
> 
>      ---> false positive, see comment #11
> 
> [x]: Package complies to the Packaging Guidelines
> [!]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses found:
>      "BSD (3 clause)", "Unknown or generated". 13 files have unknown license.
>      Detailed output of licensecheck in
>      /home/bjoern.esser/fedora/review/973084-htmlcleaner/licensecheck.txt
> 
>      ---> see above: 's!BSD!& with advertising!'
> 
> [x]: License file installed when any subpackage combination is installed.
> [x]: Package consistently uses macro is (instead of hard-coded directory
>      names).
> [x]: Package is named according to the Package Naming Guidelines.
> [x]: Package does not generate any conflict.
> [x]: Package obeys FHS, except libexecdir and /usr/target.
> [x]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [x]: Package must own all directories that it creates.
> [x]: Package does not own files or directories owned by other packages.
> [x]: Requires correct, justified where necessary.
> [x]: Spec file is legible and written in American English.
> [-]: Package contains systemd file(s) if in need.
> [-]: Large documentation must go in a -doc subpackage.
>      Note: Documentation size is 10240 bytes in 1 files.
> [x]: All build dependencies are listed in BuildRequires, except for any that
>      are listed in the exceptions section of Packaging Guidelines.
> [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.
> [x]: Each %files section contains %defattr if rpm < 4.4
> [x]: Macros in Summary, %description expandable at SRPM build time.
> [x]: Package does not contain duplicates in %files.
> [x]: Permissions on files are set properly.
> [x]: 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.
> [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
>      work.
> [x]: Package is named using only allowed ASCII characters.
> [x]: Package do not use a name that already exist
> [x]: Package is not relocatable.
> [x]: Sources used to build the package match the upstream source, as provided
>      in the spec URL.
> [x]: Spec file name must match the spec package %{name}, in the format
>      %{name}.spec.
> [x]: File names are valid UTF-8.
> [x]: Packages must not store files under /srv, /opt or /usr/local
> [x]: Package successfully compiles and builds into binary rpms on at least
> one
>      supported primary architecture.
> [x]: Package installs properly.
> [x]: Rpmlint is run on all rpms the build produces.
>      Note: No rpmlint messages.
> 
> Java:
> [x]: Packages have proper BuildRequires/Requires on jpackage-utils
>      Note: Maven packages do not need to (Build)Require jpackage-utils. It is
>      pulled in by maven-local
> [x]: Fully versioned dependency in subpackages, if present.
> [x]: Javadoc documentation files are generated and included in -javadoc
>      subpackage
> [x]: Javadoc subpackages should not have Requires: jpackage-utils
> [x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
> [x]: Bundled jar/class files should be removed before build
> 
> Maven:
> [x]: If package contains pom.xml files install it (including depmaps) even
>      when building with ant
> [x]: Pom files have correct Maven mapping
> [x]: Maven packages should use new style packaging
> [x]: Old add_to_maven_depmap macro is not being used
> [x]: Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-
>      utils for %update_maven_depmap macro
> [x]: Package DOES NOT use %update_maven_depmap in %post/%postun
> [x]: Packages use %{_mavenpomdir} instead of %{_datadir}/maven2/poms
> 
> ===== SHOULD items =====
> 
> Generic:
> [-]: 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]: Final provides and requires are sane (see attachments).
> [x]: Package functions as described.
> [x]: Latest version is packaged.
> [x]: Package does not include license text files separate from upstream.
> [-]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.
> [x]: Package should compile and build into binary rpms on all supported
>      architectures.
> [x]: %check is present and all tests pass.
> [x]: Packages should try to preserve timestamps of original installed files.
> [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
> [x]: Sources can be downloaded from URI in Source: tag
> [x]: Reviewer should test that the package builds in mock.
> [x]: Buildroot is not present
> [x]: Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
> [x]: Dist tag is present.
> [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
> [x]: SourceX tarball generation or download is documented.
> [x]: SourceX is a working URL.
> [x]: Spec use %global instead of %define.
> 
> Java:
> [x]: Package uses upstream build method (ant/maven/etc.)
> [x]: Packages are noarch unless they use JNI
> 
> ===== EXTRA items =====
> 
> Generic:
> [x]: Large data in /usr/share should live in a noarch subpackage if package
> is
>      arched.
> [x]: Rpmlint is run on all installed packages.
>      Note: No rpmlint messages.
> [x]: Spec file according to URL is the same as in SRPM.
> 
> 
> Rpmlint
> -------
> Checking: htmlcleaner-2.2.1-2.fc20.noarch.rpm
>           htmlcleaner-javadoc-2.2.1-2.fc20.noarch.rpm
> 2 packages and 0 specfiles checked; 0 errors, 0 warnings.
> 
> 
> 
> 
> Rpmlint (installed packages)
> ----------------------------
> # rpmlint htmlcleaner htmlcleaner-javadoc
> 2 packages and 0 specfiles checked; 0 errors, 0 warnings.
> # echo 'rpmlint-done:'
> 
> 
> 
> Requires
> --------
> htmlcleaner (rpmlib, GLIBC filtered):
>     java
>     jpackage-utils
>     mvn(org.jdom:jdom)
> 
> htmlcleaner-javadoc (rpmlib, GLIBC filtered):
>     jpackage-utils
> 
> 
> 
> Provides
> --------
> htmlcleaner:
>     htmlcleaner
>     mvn(net.sourceforge.htmlcleaner:htmlcleaner)
> 
> htmlcleaner-javadoc:
>     htmlcleaner-javadoc
> 
> 
> 
> Source checksums
> ----------------
> http://downloads.sourceforge.net/project/htmlcleaner/htmlcleaner/htmlcleaner
> v2.2.1/htmlcleaner-2.2.1-src.zip :
>   CHECKSUM(SHA256) this package     :
> d1f045efff57d266c94e6b87e6685c14d7fbec3aac648f6020cc69812fe0be31
>   CHECKSUM(SHA256) upstream package :
> d1f045efff57d266c94e6b87e6685c14d7fbec3aac648f6020cc69812fe0be31
> 
> 
> Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
> Buildroot used: fedora-rawhide-x86_64
> Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 973084
> 
> #####
> 
> Good work! Just fix the issue with license-tag (already mentioned in comment
> #11) and package will be pristine.

Comment 16 Björn Esser (besser82) 2013-06-21 08:35:59 UTC
(In reply to Marcin.Dulak from comment #15)
> > 's!BSD$!& with advertising!'
> 
> the /usr/share/doc/htmlcleaner-2.2.1/licence.txt file
> looks to me closer to
> https://fedoraproject.org/wiki/Licensing:BSD?rd=Licensing/BSD#3ClauseBSD
> It's right that 13 files do not contain license header, but why
> changing the license to "BSD with advertising"?

You're right of course. The part about:

 | Neither the name of the <ORGANIZATION> nor the names of its contributors
 | may be used to endorse or promote products derived from this software
 | without specific prior written permission

got me on the wrong track... ;)

Note: You possibly want to rename `licence.txt` to proper spelling, but that's up to you deciding about:

    %prep
    ....
    ln -f LICENSE licence.txt

    %doc LICENSE


Your package is fine to be imported to SCM now!

APPROVED!

Comment 17 Björn Esser (besser82) 2013-06-21 09:06:25 UTC
Just forgot:

You're doing a nice work on bz #961180

Please add me `besser82` as co-maintainer for SCM and set `InitialCC: java-sig`, please. So I can help you, if there's something related to packaging and java-oriented fedora-people get aware there's a new java-package around.

Comment 18 marcindulak 2013-06-21 09:24:15 UTC
New Package SCM Request
=======================
Package Name: htmlcleaner
Short Description: HTML parser written in Java
Owners: marcindulak besser82
Branches: f19
InitialCC: java-sig

Comment 19 Björn Esser (besser82) 2013-06-21 09:36:32 UTC
Please be careful with the flags. :)  I fixed them.  If you set fedora-review to (+) for others it looks like you granted review yourself.  But never mind this happend to me on my first packages as well. ;)

Comment 20 marcindulak 2013-06-21 10:21:51 UTC
(In reply to Björn Esser from comment #19)
> Please be careful with the flags. :)  I fixed them.  If you set
> fedora-review to (+) for others it looks like you granted review yourself. 
> But never mind this happend to me on my first packages as well. ;)

yes, i was surprised i can do that myself. Thanks for fixing that!

Comment 21 Björn Esser (besser82) 2013-06-21 10:55:59 UTC
(In reply to Marcin.Dulak from comment #20)
> yes, i was surprised i can do that myself. Thanks for fixing that!

You're welcome!

Yes, now you are a packager, you can, but mustn't do on your own packages. :)  You can take official reviews for other packagers, too.  The comments you made in [1] look really good and taking this to full review && approval sounds like a good start.

You may want to install the `fedora-review` package providing all basic stuff needed.  This is a half-automated tool checking some stuff, but may report false-positives or miss some. It doesn't check everything, so some manual work will be needed either.

An introduction about the review process you can find here. [2]  And here's [3] a list with packages waiting for review; those with green bg need review by a packager-sponsor.

[1] bz #961180
[2] https://fedoraproject.org/wiki/Packaging/ReviewGuidelines
[3] http://fedoraproject.org/PackageReviewStatus/NEW.html

Comment 22 Gwyn Ciesla 2013-06-21 12:02:45 UTC
Git done (by process-git-requests).

Comment 23 marcindulak 2013-06-21 13:45:58 UTC
(In reply to Björn Esser from comment #21)
> (In reply to Marcin.Dulak from comment #20)
> > yes, i was surprised i can do that myself. Thanks for fixing that!
> 
> You're welcome!
> 
> Yes, now you are a packager, you can, but mustn't do on your own packages.
> :)  You can take official reviews for other packagers, too.  The comments
> you made in [1] look really good and taking this to full review && approval
> sounds like a good start.
OK
> 
> You may want to install the `fedora-review` package providing all basic
> stuff needed.  This is a half-automated tool checking some stuff, but may
> report false-positives or miss some. It doesn't check everything, so some
> manual work will be needed either.
already using fedora-review, it helps a lot.
> 
> An introduction about the review process you can find here. [2]  And here's
> [3] a list with packages waiting for review; those with green bg need review
> by a packager-sponsor.
> 
> [1] bz #961180
> [2] https://fedoraproject.org/wiki/Packaging/ReviewGuidelines
> [3] http://fedoraproject.org/PackageReviewStatus/NEW.html
see already few interesting candidates for review there

Comment 24 Björn Esser (besser82) 2013-06-21 14:37:17 UTC
(In reply to Marcin.Dulak from comment #23)
> see already few interesting candidates for review there

So you may want to add yourself to CC of the bugs tracking changes:

  * go to the bug
  * check the "Add me to CC list"
  * --> "Save changes"


So, limb has setup the git-repo for importing your package.  At this point you want to make sure having 

  * installed the pkg from "fedora-packager"-group:
    `yum groups install fedora-packager`

  * have imported your rsa pub-key into your account:
    https://admin.fedoraproject.org/accounts/user/edit/marcindulak

  * have run `fedora-packager-setup` and imported the generated
    certificate into your browser, so you can login into koji:
    https://koji.fedoraproject.org/koji/index

After you have completed these steps. You should follow the instructions shown in the wiki:  http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Check_out_the_module

Some overview for advanced use of `fedpkg` is given here:
http://fedoraproject.org/wiki/Using_git_FAQ_for_package_maintainers

If you have any questions or problems feel free to ask me!

Comment 25 marcindulak 2013-06-21 19:33:07 UTC
(In reply to Björn Esser from comment #24)
> (In reply to Marcin.Dulak from comment #23)
> > see already few interesting candidates for review there
> 
> So you may want to add yourself to CC of the bugs tracking changes:
> 
>   * go to the bug
>   * check the "Add me to CC list"
>   * --> "Save changes"
> 
> 
> So, limb has setup the git-repo for importing your package.  At this point
> you want to make sure having 
> 
>   * installed the pkg from "fedora-packager"-group:
>     `yum groups install fedora-packager`
> 
>   * have imported your rsa pub-key into your account:
>     https://admin.fedoraproject.org/accounts/user/edit/marcindulak
> 
>   * have run `fedora-packager-setup` and imported the generated
>     certificate into your browser, so you can login into koji:
>     https://koji.fedoraproject.org/koji/index
> 
> After you have completed these steps. You should follow the instructions
> shown in the wiki: 
> http://fedoraproject.org/wiki/
> Join_the_package_collection_maintainers#Check_out_the_module
> 
> Some overview for advanced use of `fedpkg` is given here:
> http://fedoraproject.org/wiki/Using_git_FAQ_for_package_maintainers
> 
> If you have any questions or problems feel free to ask me!

OK. i think i have those koji builds, what to do next, 'comps'?

Comment 26 Björn Esser (besser82) 2013-06-22 06:00:42 UTC
(In reply to Marcin.Dulak from comment #25)
> OK. i think i have those koji builds, what to do next, 'comps'?

So all went fine so far and you now have ready-build pkgs for rawhide and F19.  For rawhide your pkg gets imported automaticly after it's been built.

If you're not familiar with `vim` as editor you probably want to use another, e.g. `nano`: `export EDITOR="$my_fav"`
Replace $my_fav with your prefered editor.  You can make this system-default by invoking `echo 'export EDITOR="$my_fav"' > /etc/profile.d/preferred_editor.sh` as root.

For the F19 branch you need to tell "bodhi" to push it to the repo:
http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Submit_Package_as_Update_in_Bodhi

`fedpkg switch-branch f19 ; fedpkg update`

After invoking the commands your editor opens giving you a template to complete:

[ htmlcleaner-2.2.1-2.fc19 ]

# bugfix, security, enhancement, newpackage (required)
type=                  <--- here goes the type of the update, in your case
                            it's newpackage. enhancement is meant for new
                            upstream release. bugfix and security are self
                            explaining, I think.

# testing, stable
request=testing        <--- keep the default here, directly pushing to stable
                            repo is meant for really urgent security-fixes.
                            (type=security)

# Bug numbers: 1234,9876
bugs=973084,11         <--- just delete the `,11` here, bodhi will close your
                            review-bug automatically then.  See below.

# Description of your update
notes=                 <--- Here is where you give an explanation of your
                            update.  On newpkgs I'd propose %description, on
                            enhancements or bugfixes %changelog.
                            All in a row without manual line-breaks.

# Enable request automation based on the stable/unstable karma thresholds
autokarma=True         <--- you can keep the defaults here.
stable_karma=3
unstable_karma=-3

# Automatically close bugs when this marked as stable
close_bugs=True        <--- See above, bug numbers.

# Suggest that users restart after update
suggest_reboot=False   <--- mostly needed for kernel-updates or some daemons.

After you saved the modded template and close the editor your update-request will be send to bodhi. You'll need your FAS-password during progress.

You can push your package to stable after 7 days in testing (bodhi will inform you by mail) or it gets auto-pushed when needed karma was reached (other packager-people may test your package and give +1/-1-votes).

Adding libs to comps should be avoided, see:
https://fedoraproject.org/wiki/How_to_use_and_edit_comps.xml_for_package_groups?rd=PackageMaintainers/CompsXml#When_to_Edit_comps

 | Libraries should not be included - they will be pulled in via dependencies

You surely want to add your package to Upstream Release Monitoring:
http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Watch_for_updates
https://fedoraproject.org/wiki/Upstream_release_monitoring?rd=Upstream_Release_Monitoring

If your package is added there, you'll get a new bug opened on bugzilla informing you by mail, everytime there's a new upstream version released.

Comment 27 Fedora Update System 2013-06-22 10:00:23 UTC
htmlcleaner-2.2.1-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/htmlcleaner-2.2.1-2.fc19

Comment 28 Fedora Update System 2013-06-22 18:15:30 UTC
htmlcleaner-2.2.1-2.fc19 has been pushed to the Fedora 19 testing repository.

Comment 29 Fedora Update System 2013-07-22 00:34:55 UTC
htmlcleaner-2.2.1-2.fc19 has been pushed to the Fedora 19 stable repository.


Note You need to log in before you can comment on or make changes to this bug.