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: icu4j https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199592 vivekl@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|vivekl@xxxxxxxxxx |mwringe@xxxxxxxxxx Flag| |fedora-review- ------- Additional Comments From vivekl@xxxxxxxxxx 2007-02-12 16:18 EST ------- X suggests the subsection needs attention + is a positive comment . is a specific comment about a problem MUST: X * package is named appropriately - match upstream tarball or project name + OK - try to match previous incarnations in other distributions/packagers for consistency + OK - 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 . 0:3.4.5-2jpp.1 -> 0:3.4.5-2jpp.2%{?dist} to be inline with http://fedoraproject.org/wiki/PackagingDrafts/ExceptionJPackage - 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 + OK * is it legal for Fedora to distribute this? + OSI-approved X licence OSI approved - 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 X * license field matches the actual license. + The license according http://www-306.ibm.com/software/globalization/icu/license.jsp is X License * license is open source-compatible. - use acronyms for licences where common + Yes but needs to be confirmed to be the correct license, see above. * specfile name matches %{name} + OK. * verify source and patches (md5sum matches upstream, know what the patches do) + OK. The patches use windows style CRLF, please use sed to fix. - 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) . Use the buildroot specified above X * if %{?dist} is used, it should be in that form (note the ? and % locations) . Use the new naming convention mentioned above * 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 rpm gives no output W: icu4j 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". W: icu4j wrong-file-end-of-line-encoding /usr/share/doc/icu4j-3.4.5/license.html This file has wrong end-of-line encoding, usually caused by creation or modification on a non-Unix system. It could prevent it from being displayed correctly in some circumstances. W: icu4j wrong-file-end-of-line-encoding /usr/share/doc/icu4j-3.4.5/APIChangeReport.html This file has wrong end-of-line encoding, usually caused by creation or modification on a non-Unix system. It could prevent it from being displayed correctly in some circumstances. W: icu4j wrong-file-end-of-line-encoding /usr/share/doc/icu4j-3.4.5/readme.html This file has wrong end-of-line encoding, usually caused by creation or modification on a non-Unix system. It could prevent it from being displayed correctly in some circumstances. W: icu4j-eclipse non-standard-group Text Editors/Integrated Development Environments (IDE) 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". W: icu4j-eclipse no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files. . There should probably be an EPL file in the eclipse subpackage that needs to be added W: icu4j-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". W: icu4j-javadoc dangerous-command-in-%post rm + If you get rid of the script driven javadoc handling, you wont have to deal with this, see below W: icu4j 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". W: icu4j mixed-use-of-spaces-and-tabs (spaces: line 9, tab: line 55) The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. - justify warnings if you think they shouldn't be there + Groups can be ignored . Fix the end line encoding . Add EPL documentation to eclipse package * 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/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 + OK * package successfully compiles and builds on at least x86 + ?Builds OK locally * BuildRequires are proper + Builds in mock - builds in mock will flush out problems here - the following packages don't need to be listed in BuildRequires: bash bzip2 coreutils cpio diffutils fedora-release (and/or redhat-release) gcc gcc-c++ gzip make patch perl redhat-rpm-config rpm-build sed tar unzip which * summary should be a short and concise description of the package + OK * description expands upon summary (don't include installation instructions) + OK * make sure lines are <= 80 characters + Everything except the gc_support incantation seems OK, if possible reformat * specfile written in American English + OK * make a -doc sub-package if necessary + OK, the javadoc subpackages are equivalent to this * - 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 + None of the above apply * should the package contain a -devel sub-package? + N/A X use macros appropriately and consistently - ie. %{buildroot} and %{optflags} vs. $RPM_BUILD_ROOT and $RPM_OPT_FLAGS - $RPM_BUILD_ROOT and %{buildroot} used interchangably * don't use %makeinstall * locale data handling correct (find_lang) - if translations included, add BR: gettext and use %find_lang %{name} at the end of %install X consider using cp -p to preserve timestamps Some cp commands not using -p option, suggest adding them if possible * split Requires(pre,post) into two separate lines + N/A * package should probably not be relocatable + Not relocatable * package contains code - see http://fedoraproject.org/wiki/Packaging/Guidelines#CodeVsContent + OK - in general, there should be no offensive content + To the best of my knowledge no ofensive content :) X package should own all directories and files . /usr/lib/eclipse should be owned by libswt3-gtk2 in the latest update to it, add a require for it . jpackage-utils is needed for the javadoc and base package since it needs /usr/share/java{,doc}. Please take a look at https://zarb.org/pipermail/jpackage-discuss/2007-February/011119.html and modify the javadoc handling appropriately. If you use the above javadoc handling then you can limit to Requires: jpackage-utils in both javadoc and main packages, o/w you need Requires(post) and Requires on jpackage-utils as well as Requires(post) on rm and ln in javadoc package and a requires on the main package for jpackage-utils * there should be no %files duplicates + OK * file permissions should be okay; %defattrs should be present + OK * %clean should be present + OK * %doc files should not affect runtime + OK * if it is a web apps, it should be in /usr/share/%{name} and *not* /var/www + Not a web app X verify the final provides and requires of the binary RPMs + Builds in mock fine . Requires need to be fixed, check "package should own all directories and files" SHOULD: * package should include license text in the package and mark it with %doc + OK * package should build on i386 + Builds in 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 on the CC list for the bug, or are watching someone who is. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review