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: opengrok - A wicked fast source browser Alias: opengrok-review https://bugzilla.redhat.com/show_bug.cgi?id=433312 ------- Additional Comments From overholt@xxxxxxxxxx 2008-04-11 14:38 EST ------- Created an attachment (id=302154) --> (https://bugzilla.redhat.com/attachment.cgi?id=302154&action=view) specfile patch to remove gcj bits Here's the full review done by Deepak Bhole and myself. It's largely fine. Thanks for the submission. For now, see the lines beginning with an "X" and the notes section at the bottom. MUST: * package is named appropriately * it is legal for Fedora to distribute this? * license field matches the actual license. * license is open source-compatible. * specfile name matches %{name} X verify source and patches - md5sums match. see notes section for comments on patches * summary and description fine * correct buildroot * %{?dist} used properly * license text included in package and marked with %doc * packages meets FHS (http://www.pathname.com/fhs/) * rpmlint on <this package>.srpm gives no output * changelog fine * Packager tag should not be used * Vendor tag should not be used * Distribution tag should not be used * use License and not Copyright * Summary tag should not end in a period * if possible, replace PreReq with Requires(pre) and/or Requires(post) X specfile is legible - I'd rather see common_reqs split out and enumerated in both Requires and BuildRequires * package successfully compiles and builds on at least x86 * BuildRequires are proper * summary should be a short and concise description of the package * description expands upon summary * make sure lines are <= 80 characters - some code lines longer; okay * specfile written in American English * no -doc sub-package necessary * no static libraries * no rpath * config files should usually be marked with %config(noreplace) * GUI apps should contain .desktop files * should the package contain a -devel sub-package? - nope * use macros appropriately and consistently * don't use %makeinstall * install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot} * locale data handling correct (find_lang) X consider using cp -p to preserve timestamps - please use cp -pR in %install * split Requires(pre,post) into two separate lines * package contains code * %files okay * %clean should be present * %doc files should not affect runtime X verify the final provides and requires of the binary RPMs - the -javadoc package should depend upon jpackage-utils (which owns %{_javadocdir} ... yes, this should probably be in the guidelines :) [localhost:SPECS]$ rpm -q --provides -p ../RPMS/noarch/opengrok-0.6-9.hg275.fc9.noarch.rpm config(opengrok) = 0.6-9.hg275.fc9 opengrok = 0.6-9.hg275.fc9 [localhost:SPECS]$ rpm -q --requires -p ../RPMS/noarch/opengrok-0.6-9.hg275.fc9.noarch.rpm /bin/sh ant bcel config(opengrok) = 0.6-9.hg275.fc9 ctags jakarta-oro java javacc jpackage-utils lucene > 2 lucene-contrib > 2 rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 servlet swing-layout [localhost:SPECS]$ rpm -q --requires -p ../RPMS/noarch/opengrok-javadoc-0.6-9.hg275.fc9.noarch.rpm rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 [localhost:SPECS]$ rpm -q --provides -p ../RPMS/noarch/opengrok-javadoc-0.6-9.hg275.fc9.noarch.rpm opengrok-javadoc = 0.6-9.hg275.fc9 * run rpmlint on the binary RPMs $ rpmlint ../RPMS/noarch/opengrok-0.6-9.hg275.fc9.noarch.rpm opengrok.noarch: W: uncompressed-zip /usr/share/java/opengrok-0.6.jar - this is fine SHOULD: * package should include license text in the package and mark it with %doc * package should build on i386 ? package should build in mock - I didn't try Notes: - don't build with gcj at all because of this missing bit: [javac] import java.util.Scanner; [javac] ^^^^^^^^^^^^^^^^^ [javac] The import java.util.Scanner cannot be resolved - remove gcj bits as the diff I'm attaching does - re-name patches to match version (0.5 -> 0.6) - for now, remove the tomcat package as we don't have guidelines -- or even best practices -- for this - why don't you build a jrcs package and Require/BR it? - please comment the patches that don't have comments in them - why the big patch between 0.6 and this hg snapshot? if that's actually required, why not just use an hg snapshot tarball as SOURCE0 instead of 0.6 and patching? -- Configure bugmail: https://bugzilla.redhat.com/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