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: dtdparser-1.21-3jpp - A Java DTD Parser https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=227050 vivekl@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|vivekl@xxxxxxxxxx |tbento@xxxxxxxxxx Flag|fedora-review? |fedora-review- ------- Additional Comments From vivekl@xxxxxxxxxx 2007-02-12 17:26 EST ------- X suggests the subsection needs attention + is a positive comment . is a specific comment about a problem MUST: X * package is named appropriately . 0:3.4.5-2jpp.1 -> 0:3.4.5-2jpp.2%{?dist} to be inline with http://fedoraproject.org/wiki/PackagingDrafts/ExceptionJPackage - match upstream tarball or project name + MD5SUMs match - try to match previous incarnations in other distributions/packagers for consistency + Consistent with JPackage - specfile should be %{name}.spec + OK - non-numeric characters should only be used in Release (ie. cvs or something) + OK - for non-numerics (pre-release, CVS snapshots, etc.), see http://fedoraproject.org/wiki/Packaging/NamingGuidelines#PackageRelease + N/A - if case sensitivity is requested by upstream or you feel it should be not just lowercase, do so; otherwise, use all lower case for the name + N/A * is it legal for Fedora to distribute this? - OSI-approved + LGPL OK. - not a kernel module - not shareware - is it covered by patents? - it *probably* shouldn't be an emulator - no binary firmware + None of these apply * license field matches the actual license. + OK * license is open source-compatible. - use acronyms for licences where common + OK * specfile name matches %{name} + OK * verify source and patches (md5sum matches upstream, know what the patches do) + No patches, MD5 OK - if upstream doesn't release source drops, put *clear* instructions on how to generate the the source drop; ie. # svn export blah/tag blah # tar cjf blah-version-src.tar.bz2 blah + N/A * skim the summary and description for typos, etc. + OK X correct buildroot - should be: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) X if %{?dist} is used, it should be in that form (note the ? and % locations) . See above about naming convention * license text included in package and marked with %doc + OK * keep old changelog entries; use judgement when removing (too old? useless?) + N/A * packages meets FHS (http://www.pathname.com/fhs/) + OK X* rpmlint on <this package>.srpm and rpms gives no output - justify warnings if you think they shouldn't be there W: dtdparser non-standard-group Development/Libraries/Java The value of the Group tag in the package is not valid. Valid groups are: "Amusements/Games", "Amusements/Graphics", "Applications/Archiving", "Applications/Communications", "Applications/Databases", "Applications/Editors", "Applications/Emulators", "Applications/Engineering", "Applications/File", "Applications/Internet", "Applications/Multimedia", "Applications/Productivity", "Applications/Publishing", "Applications/System", "Applications/Text", "Development/Debug", "Development/Debuggers", "Development/Languages", "Development/Libraries", "Development/System", "Development/Tools", "Documentation", "System Environment/Base", "System Environment/Daemons", "System Environment/Kernel", "System Environment/Libraries", "System Environment/Shells", "User Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support". E: dtdparser tag-not-utf8 %changelog The character encoding of the value of this tag is not UTF-8. . use iconv to convert to UTF8 E: dtdparser non-utf8-spec-file dtdparser.spec The character encoding of the spec file is not UTF-8. Convert it for example using iconv(1). . use iconv to convert to UTF8 W: dtdparser mixed-use-of-spaces-and-tabs (spaces: line 9, tab: line 36) The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. . Replace the tabs with spaces (:set tabexpand :%retab in vim) W: dtdparser non-standard-group Development/Libraries/Java The value of the Group tag in the package is not valid. Valid groups are: "Amusements/Games", "Amusements/Graphics", "Applications/Archiving", "Applications/Communications", "Applications/Databases", "Applications/Editors", "Applications/Emulators", "Applications/Engineering", "Applications/File", "Applications/Internet", "Applications/Multimedia", "Applications/Productivity", "Applications/Publishing", "Applications/System", "Applications/Text", "Development/Debug", "Development/Debuggers", "Development/Languages", "Development/Libraries", "Development/System", "Development/Tools", "Documentation", "System Environment/Base", "System Environment/Daemons", "System Environment/Kernel", "System Environment/Libraries", "System Environment/Shells", "User Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support". E: dtdparser tag-not-utf8 %changelog The character encoding of the value of this tag is not UTF-8. . use iconv to convert to UTF8 W: dtdparser-javadoc non-standard-group Development/Documentation The value of the Group tag in the package is not valid. Valid groups are: "Amusements/Games", "Amusements/Graphics", "Applications/Archiving", "Applications/Communications", "Applications/Databases", "Applications/Editors", "Applications/Emulators", "Applications/Engineering", "Applications/File", "Applications/Internet", "Applications/Multimedia", "Applications/Productivity", "Applications/Publishing", "Applications/System", "Applications/Text", "Development/Debug", "Development/Debuggers", "Development/Languages", "Development/Libraries", "Development/System", "Development/Tools", "Documentation", "System Environment/Base", "System Environment/Daemons", "System Environment/Kernel", "System Environment/Libraries", "System Environment/Shells", "User Interface/Desktops", "User Interface/X", "User Interface/X Hardware Support". + All group warnings can be ignored. E: dtdparser-javadoc tag-not-utf8 %changelog The character encoding of the value of this tag is not UTF-8. . use iconv to convert to UTF8 E: dtdparser-javadoc zero-length /usr/share/javadoc/dtdparser-1.21/package-list + I checked the build root on a local build and this seems to be created by the javadoc task in ant. This can probably be ignored? * changelog should be in one of these formats: * Fri Jun 23 2006 Jesse Keating <jkeating@xxxxxxxxxx> - 0.6-4 - And fix the link syntax. * Fri Jun 23 2006 Jesse Keating <jkeating@xxxxxxxxxx> 0.6-4 - And fix the link syntax. * Fri Jun 23 2006 Jesse Keating <jkeating@xxxxxxxxxx> - 0.6-4 - And fix the link syntax. + OK * Packager tag should not be used + OK * Vendor and distribution tag should not be used + OK * use License and not Copyright + OK * Summary tag should not end in a period + OK * if possible, replace PreReq with Requires(pre) and/or Requires(post) + N/A * specfile is legible - this is largely subjective; use your judgement * package successfully compiles and builds on at least x86 * BuildRequires are proper + Seems OK, built on mock - builds in mock will flush out problems here * summary should be a short and concise description of the package + OK * description expands upon summary (don't include installation instructions) + OK X make sure lines are <= 80 characters . minor fixes needed * specfile written in American English + OK X make a -doc sub-package if necessary Standardize the javadoc package handling around https://zarb.org/pipermail/jpackage-discuss/2007-February/011119.html - see http://fedoraproject.org/wiki/Packaging/Guidelines#head-9bbfa57478f0460c6160947a6bf795249488182b * packages including libraries should exclude static libraries if possible * don't use rpath * config files should usually be marked with %config(noreplace) * GUI apps should contain .desktop files + The above dont apply * should the package contain a -devel sub-package? + N/A * use macros appropriately and consistently - ie. %{buildroot} and %{optflags} vs. $RPM_BUILD_ROOT and $RPM_OPT_FLAGS + RPM_BUILD_ROOT seems to be used consistently * don't use %makeinstall + N/A * locale data handling correct (find_lang) - if translations included, add BR: gettext and use %find_lang %{name} at the end of %install + N/A * consider using cp -p to preserve timestamps + OK * split Requires(pre,post) into two separate lines + None used yet * package should probably not be relocatable + Non relocatable * package contains code - see http://fedoraproject.org/wiki/Packaging/Guidelines#CodeVsContent - in general, there should be no offensive content + OK X* package should own all directories and files + Use jpackage-utils in Requires(x), Requires since installing to %{_javadir}/%{_javadocdir} * there should be no %files duplicates + OK * file permissions should be okay; %defattrs should be present + OK * %clean should be present + OK X* %doc files should not affect runtime . javadoc should use %doc for its files * if it is a web apps, it should be in /usr/share/%{name} and *not* /var/www + Not a webapp X* verify the final provides and requires of the binary RPM rpm -qp --provides ../RPMS/noarch/dtdparser-* dtdparser = 0:1.21-3jpp dtdparser-javadoc = 0:1.21-3jpp rpm -qp --requires ../RPMS/noarch/dtdparser-* rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 . Requires needs jpakage-utils as mentioned earlier . Should have a requires on java? SHOULD: * package should include license text in the package and mark it with %doc + OK * package should build on i386 + Builds on mock * package should build in mock + OK -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review