[Bug 227075] Review Request: jtidy-1.0-0.20000804r7dev.6jpp - HTML syntax checker and pretty printer

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: jtidy-1.0-0.20000804r7dev.6jpp - HTML syntax checker and pretty printer


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


overholt at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|nsantos at redhat.com          |fnasser at redhat.com




------- Additional Comments From overholt at redhat.com  2007-02-12 17:08 EST -------
The spec that is attached still has the double Requires/missing BR problem.

Here is the review for the attached specfile and
http://people.redhat.com/fnasser/jtidy-1.0-0.20000804r7dev.8jpp.src.rpm :

Still remaining (things requiring fixes being with X):

X rpmlint on jtidy srpm gives no output

W: jtidy non-standard-group Text Processing/Markup/HTML

. ignore this one

E: jtidy unknown-key GPG#c431416d

. ignore this.  it's just the JPackage GPG key.

E: jtidy tag-not-utf8 %changelog
E: jtidy non-utf8-spec-file jtidy.spec

. I think this *might* be the accent in Ville's last name
. run:  iconv -f iso-8859-1 -t utf-8 -o

* package is named appropriately
* specfile name matches %{name}
X BuildRoot incorrect.  Should be:

%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

X remove "section free"
? why have the scripts sub-package at all?  I think we should just put
jtidy.script into the main jtidy package.  This should be done at JPackage,
though, I guess, so don't worry about it here.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package and marked with %doc
* specfile written in American English
* specfile is legible
? I think the script should be renamed to just %{name}.script
? why use %__rm and not just rm?
? same for %__chmod, %ant, %__sed, and %__ln_s
-> just nit-picks and not something that will hold up the review
* source files match upstream (md5sum checked)
X package successfully compiles and builds on at least x86
. I get a whole bunch of these errors using the latest gcj 4.1 branch (with the
generics backport):

    [javac] 26. ERROR in
/home/andrew/rpmbuild/BUILD/jtidy-04aug2000r7-dev/src/org/w3c/tidy/DOMElementImpl.java
(at line 31)
    [javac]     public class DOMElementImpl extends DOMNodeImpl
    [javac]                  ^^^^^^^^^^^^^^
    [javac] The type DOMElementImpl must implement the inherited abstract method
Element.setIdAttribute(String, boolean)

X BuildRequires are proper
. one of the duplicate "Requires: xml-commons-apis" should become a BuildRequires
* no locale data so no find_lang necessary
* package is not relocatable
* package owns all directories and files
* no %files duplicates
* file permissions are fine; %defattrs present
* %clean present
* macro usage is consistent
* package contains code
* no large docs so no -doc subpackage
. javadoc package present
* %doc files don't affect runtime
* shared libraries are present, but no ldconfig required.
* no pkgconfig or header files
* no -devel package
* no .la files
* no desktop file
* not a web app.
* file ownership fine
* final provides and requires are sane

SHOULD:
* package includes license text
X package builds on i386
. see above
X package functions
  . I don't know how to test this package
X package builds in mock
  my mock setup doesn't seem to be working but I'll try to test on Monday


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.