[Bug 246138] Review Request: eclipse-QuickREx - QuickREx is a regular-expression test Eclipse Plug-In

bugzilla at redhat.com bugzilla at redhat.com
Wed Jul 11 18:42:23 UTC 2007


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

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




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




More information about the Fedora-package-review mailing list