Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=484725 Andrew Overholt <overholt@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |overholt@xxxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |overholt@xxxxxxxxxx Flag| |fedora-review? --- Comment #1 from Andrew Overholt <overholt@xxxxxxxxxx> 2009-02-10 11:17:11 EDT --- Everything looks pretty good. The Summary and Description need a bit of attention. For the summary I'd recommend something like: "An Eclipse plugin that assists with writing more unit tests" and for the description something like: "MoreUnit is an Eclipse plugin that should assist with writing more unit tests. It can decorate classes which have testcase(s) and mark methods in the editor which are under test. Renaming/moving classes/methods will cause moreUnit to rename/move the corresponding test code. Generation of test method stubs is also possible." Here's the rest of the review. Lines beginning with non-* need attention. X make sure lines are <= 80 characters - the summary line is longer but will be fixed if you change it to something similar to what I suggest above X changelog format okay - you probably want to use your actual email address :) X skim the summary and description for typos, etc. - see above ? license text included in package and marked with %doc - could we speak with upstream about getting them to include a copy of the EPL text? Not a blocker. X run rpmlint on the binary RPMs => no output $ rpmlint ../RPMS/noarch/eclipse-moreunit-1.2.0-1.fc10.noarch.rpm eclipse-moreunit.noarch: W: no-documentation To fix this, you could probably mark this (or both files in this directory) as %doc: eclipse-moreunit-fetched-src-V_1_2_0/org.moreunit.plugin/help/documentation.html * BuildRequires are proper * macros fine * 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} * md5sum matches upstream - timestamp differences exist in my generated tarball but no content differences exist * correct buildroot * %{?dist} used correctly * packages meets FHS (http://www.pathname.com/fhs/) * rpmlint on <this package>.srpm gives no output * Summary tag does not end in a period * no PreReq * specfile is legible * package successfully compiles and builds on x86_64 (but is correctly noarch) * summary and description fine * specfile written in American English * no -doc sub-package necessary * not native, so no rpath, static linking, etc. * no config files * not a GUI app * no -devel necessary * install section begins with rm -rf $RPM_BUILD_ROOT or %{buildroot} * no translations so no locale handling * no Requires(pre,post) * package not relocatable * package contains code * package owns all directories and files * no %files duplicates * file permissions fine * %clean present * %doc files do not affect runtime - they won't ... * not a web app * verify the final provides and requires of the binary RPMs - these look good to me -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review