[Bug 232710] Review Request: eclipse-sdk-nls - Eclipse language packs for eclipse-sdk

[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-sdk-nls - Eclipse language packs for eclipse-sdk


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=232710


overholt at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |overholt at redhat.com
  BugsThisDependsOn|                            |232709
               Flag|                            |fedora-review-




------- Additional Comments From overholt at redhat.com  2007-03-19 17:16 EST -------
A few notes:

. Change the URL to eclipse.org since they're providing the translations
. You don't need to BuildRequire eclipse-rcp or eclipse-platform if
eclipse-nlspackager Requires eclipse-platform
. Add a Requires:  eclipse-rcp
. Summary:  "Eclipse language packs for eclipse-sdk" -> "Eclipse language packs
for the Eclipse SDK"
. Version should match that of the Eclipse SDK base version for which the
translations were done ... 3.2.1 in this case
. Description:  "This eclipse-sdk-nls package contains multiple language
translations for Eclipse SDK." -> "This package contains multiple language
translations for the Eclipse SDK.
. "Portuguese(and Brazil)" -> "Portuguese (and Brazilian Portuguese)"
. "Chinese(Simplified and Traditional)" -> "Chinese (Simplified and Traditional)"
. dump eclipse_name and just use "eclipse" in eclipse_base's definition
. Development/Languages is incorrect but just ignore that for now

Full review text with a few more little things below:

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 (md5sum matches upstream, know what the patches do)
 - the md5sums of NLpack1-eclipse-SDK-3.2.1-gtk.zip and
   NLpack2a-eclipse-SDK-3.2.1-gtk.zip don't match my existing downloads from
   upstream.  I'm re-downloading to verify.  The other two are fine.
X skim the summary and description for typos, etc.
  -> see comments above
* correct buildroot
* %{?dist} used properly
* license text included in package and marked with %doc
* packages meets FHS (http://www.pathname.com/fhs/)
* rpmlint on eclipse-sdk-nls-0.1.0-3.src.rpm gives no output
* changelog format fine
* Packager tag not used
* Vendor tag not used
* Distribution tag not used
* License and not Copyright used
* Summary tag should not end in a period
* no PreReq
X specfile is legible
 - see comments above.
* package successfully compiles and builds on at least x86
X BuildRequires are proper
  - see comments above
X summary and description are fine
  - see comments above
X make sure lines are <= 80 characters
  - two of the lines in the nlspackager app call have extra spaces that put
    them over 80 characters
* specfile written in American English
* no -doc sub-package necessary
* no libraries
* no rpath
* no config files
* not a GUI app
* no -devel subpackage necessary
* macros used appropriately and consistently
* %makeinstall not used
* no locale data in the traditional sense
* cp -p used
* no Requires(pre,post)
* package is not relocatable
* package contains acceptable content
X package owns all directories and files
  - package needs to Require eclipse-rcp
* no %files duplicates
* file permissions fine
* %clean present
* %doc files do not affect runtime
* not a web app
? verify the final provides and requires of the binary RPMs
  . I have yet to do this
? run rpmlint on the binary RPMs
  . I have yet to do this

SHOULD:
* package should include license text in the package and mark it with %doc
* package should build on i386
? package should build in mock
  . haven't tried but I don't anticipate any problems

-- 
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.