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

bugzilla at redhat.com bugzilla at redhat.com
Sun Feb 11 00:13:30 UTC 2007


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
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |nsantos at redhat.com
                 CC|                            |overholt at redhat.com
               Flag|                            |fedora-review-




------- Additional Comments From overholt at redhat.com  2007-02-10 19:13 EST -------
MUST:
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

. I don't where this is coming from.  Perhaps the SRPM just needs to be
rebuilt on Fedora?

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

* package is named appropriately
X specfile name matches %{name}
. the specfile should just be jtidy.spec
X package meets packaging guidelines.

. BuildRoot incorrect.  As per this:

http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot

it should be:

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

. remove "section free"
. remove BuildArch
. 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 ... but this is
. why use %__rm and not just rm?
. same for %__chmod, %ant, %__sed, and %__ln_s
-> just a nit-pick 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
X no %files duplicates
. I don't think the %ghost is necessary for the last entry in %files javadoc
* 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

Note:  we should try to gcj-ify this package while we're at it.

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.




More information about the Fedora-package-review mailing list