[Bug 232709] Review Request: eclipse-nlspackager - Eclipse NLS package generator

[bugzilla at redhat.com]

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 at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review-




------- Additional Comments From fitzsim at redhat.com  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.