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: eclipse-QuickREx - QuickREx is a regular-expression test Eclipse Plug-In https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246138 overholt@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |overholt@xxxxxxxxxx Flag| |fedora-review? ------- Additional Comments From overholt@xxxxxxxxxx 2007-07-11 14:42 EST ------- Hi Alphonse, I've finished the review. Lines prefixed with a '?' are where I have a question. Those beginning with a '*' are fine and those marked with an 'X' indicate they must be fixed. The 'MUST' and 'SHOULD' headers just reflect the sections here: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines?action=show&redirect=PackageReviewGuidelines MUST: ? package is named appropriately - can we get confirmation from upstream about the capitalization issue? I just don't want to go against their wishes. Otherwise, it's fine. * license field matches the actual license. * license is open source-compatible * specfile name matches %{name} X verify source and patches (md5sum matches upstream, know what the patches do) - while I can't verify the md5sum of your tarball, I don't get any differences on a diff of the exploded tarball so I think we're fine. The instructions are also clear. - my only concern is the build.properties and feature.xml files -- did upstream author these or did you? can they not be included upstream? I thought package-build worked fine with packages that didn't have features - does it not? I guess I just want to know what the purpose of these files is and whether or not they will go upstream at some point :) . * no typos in the summary or description * buildroot fine, although this is now the most recommended value: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) * %{?dist} used properly * license text included in package and marked with %doc * packages meets FHS (http://www.pathname.com/fhs/) X rpmlint on <this package>.srpm gives no output $ rpmlint ../SRPMS/eclipse-quickrex-3.5.0-2.fc7.src.rpm eclipse-quickrex.src:145: W: strange-permission fetch-quickrex.sh 0764 Can we make it 0755 or something? X changelog fine except for extra space in first line: * Thu Jul 5 2007 Alphonse Van Assche <alcapcom@xxxxxxxxx> 3.5.0-2 ^ * Packager tag not used * Vendor tag not used * Distribution tag should not be used * use License and not Copyright * Summary tag does not end in a period * no PreReq * specfile is legible * 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 - lines that are > 80 are necessary IMO * specfile written in American English * no -doc sub-package necessary * no static libraries * no rpath * no config files * not a GUI app * no -devel sub-package necessary X macros used appropriately and consistently - %{buildroot} and $RPM_BUILD_ROOT -- pick one :) * no %makeinstall * install section begins with rm -rf $RPM_BUILD_ROOT or %{buildroot} * no locale data X consider using cp -p to preserve timestamps * Requires(pre,post) split into two separate lines * package not relocatable * package contains code and documentation * package owns all directories and files * no %files duplicates * file permissions okay; %defattrs present * %clean present * %doc files do not affect runtime * not a web app * final provides and requires of the binary RPMs fine $ rpm -qp --provides ../RPMS/i386/eclipse-quickrex-3.5.0-2.fc7.i386.rpm QuickREx.jar.so eclipse-QuickREx = 3.5.0-2.fc7 eclipse-quickrex = 3.5.0-2.fc7 $ rpm -qp --requires ../RPMS/i386/eclipse-quickrex-3.5.0-2.fc7.i386.rpm /bin/sh /bin/sh eclipse-platform >= 3.2.1 jakarta-oro java-gcj-compat java-gcj-compat libc.so.6 libc.so.6(GLIBC_2.1.3) libdl.so.2 libgcc_s.so.1 libgcc_s.so.1(GCC_3.0) libgcj_bc.so.1 libm.so.6 libpthread.so.0 librt.so.1 libz.so.1 regexp rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(VersionedDependencies) <= 3.0.3-1 rtld(GNU_HASH) * rpmlint output when run on the binary RPMs $ rpmlint ../RPMS/i386/eclipse-quickrex-3.5.0-2.fc7.i386.rpm eclipse-quickrex.i386: W: dangling-symlink /usr/share/eclipse/plugins/de.babe.eclipse.plugins.QuickREx_3.5.0/lib/jakarta-regexp-1.4.jar /usr/share/java/regexp.jar eclipse-quickrex.i386: W: symlink-should-be-relative /usr/share/eclipse/plugins/de.babe.eclipse.plugins.QuickREx_3.5.0/lib/jakarta-regexp-1.4.jar /usr/share/java/regexp.jar eclipse-quickrex.i386: W: dangling-symlink /usr/share/eclipse/plugins/de.babe.eclipse.plugins.QuickREx_3.5.0/lib/jakarta-oro-2.0.8.jar /usr/share/java/jakarta-oro-2.0.8.jar eclipse-quickrex.i386: W: symlink-should-be-relative /usr/share/eclipse/plugins/de.babe.eclipse.plugins.QuickREx_3.5.0/lib/jakarta-oro-2.0.8.jar /usr/share/java/jakarta-oro-2.0.8.jar - I think these are fine and I've never been told otherwise :). 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 but I don't anticipate any problems. Alphonse, can you try this? * package functions as expected (as far as I can tell) -- 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