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-nlspackager - Eclipse NLS package generator https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=232709 fitzsim@xxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review- ------- Additional Comments From fitzsim@xxxxxxxxxx 2007-03-16 18:31 EST ------- Some of my comments apply to the upstream project; since you are the upstream maintainer, it's best to just fix the problems there. The gcj_support macro usually controls native-compilation support. If you just want to build noarch then remove the gcj_support stuff and leave this line: BuildRequires: java-devel >= 1.4.2 That said, it's preferable to add full support for native compilation: http://fedoraproject.org/wiki/NativeJava MUST: * package is named appropriately * it is legal for Fedora to distribute this X license field matches the actual license. - the distributed zip file should contain a license and each source file should likely have a license header - it's cleaner if the zip file expands to a versioned directory, in this case: nlspackager-0.1.2 * license is open source-compatible. * specfile name matches %{name} ? source and patches verified - where does the zip file come from? I couldn't find a link to it from http://wiki.eclipse.org/index.php/Linux_Distributions_Project * summary and description okay - the description could be clearer; try to eliminate the this/that options - there's a trailing space in the Summary field * correct buildroot * %{?dist} used properly X 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 /home/fitzsim/rpmbuild/SRPMS/eclipse-nlspackager-0.1.2-1.src.rpm W: eclipse-nlspackager non-standard-group Translations - use a standard group (see output of rpmlint -i on this package) $ rpmlint /home/fitzsim/rpmbuild/RPMS/noarch/eclipse-nlspackager-0.1.2-1.noarch.rpm W: eclipse-nlspackager non-standard-group Translations W: eclipse-nlspackager no-documentation - include ChangeLog and LICENSE and probably a README file, marked with %doc X changelog fine - trailing space at the end of the %changelog entry * Packager tag not used * Vendor tag not used * Distribution tag not used * License and not Copyright used * Summary tag does not end in a period * if possible, replace PreReq with Requires(pre) and/or Requires(post) X specfile is legible - why is the copy-platform line commented out? this should be removed or explained - pushd blocks are easier to read if they're indented: pushd $RPM_BUILD_ROOT%{eclipse_base} mkdir plugins popd - the install section should just use install commands rather than mkdir, pushd, popd, cp - there's no need for the globbing in the %files section; just specify the single file, org.eclipse.linuxtools.nlspackager_%{version}.jar * package successfully compiles and builds on at least x86 X BuildRequires are proper - it would be nice if there were an explicit BuildRequires line for eclipse-platform, which provides the eclipse binary (even though eclipse-platform is pulled in by eclipse-rcp) * summary is a short and concise description of the package * description expands upon summary * make sure lines are <= 80 characters - a few lines in the %build and %install sections should be wrapped - you may want to use two-space indentation rather than tabs * specfile written in American English * no -doc sub-package necessary * no static libs * no rpath * config files should marked with %config(noreplace) * not a GUI app * sub-packages fine * macros used appropriately and consistently - ie. %{buildroot} and %{optflags} vs. $RPM_BUILD_ROOT and $RPM_OPT_FLAGS * %makeinstall not used * no locale data * Requires(pre,post) fine * package not relocatable * package contains code * package owns all directories and files * no %files duplicates X file permissions okay; %defattrs present - take out the space before the first root * %clean present * %doc files do not affect runtime * not a webapp * verify the final provides and requires of the binary RPMs SHOULD: X package should include license text in the package and mark it with %doc * package should build on i386 * package should build in mock -- 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