[Bug 269421] Review Request: eclipse-egit - Eclipse Git plugin

bugzilla at redhat.com bugzilla at redhat.com
Fri Aug 31 19:14:46 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-egit - Eclipse Git plugin


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


overholt at redhat.com changed:

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




------- Additional Comments From overholt at redhat.com  2007-08-31 15:14 EST -------
I'll take this.  Things are generally pretty good.  There are just a few minor
things (lines beginning with NEEDS_FIX) and some questions (lines beginning with ?):

MUST items:

OK package is named appropriately
OK is it legal for Fedora to distribute this?
? license field matches the actual license.
 - it says in the git web repo that some of it is LGPL ... but I can't see
   what parts - can you?  I'm okay with the dual GPLv2 and EPL as that's what
   I can see.
OK license is open source-compatible.
OK specfile name matches %{name}
? verify source and patches (md5sum matches upstream, know what the patches do)
 - I can't get the same md5sum but the contents are the same.  Did you use wget?
OK skim the summary and description for typos, etc.
OK correct buildroot
OK if %{?dist} is used, it should be in that form (note the ? and % locations)
OK license text included in package and marked with %doc
 - license text included in jar so can't mark as %doc
OK packages meets FHS (http://www.pathname.com/fhs/)
OK rpmlint on <this package>.srpm gives no output
 $ rpmlint eclipse-egit-0.2.2-0.git20070826.fc8.src.rpm 
 W: eclipse-egit invalid-license EPL GPLv2

 - this is fine since it's dual-licensed software

OK changelog should be in one of these formats:
 [...]
OK Packager tag should not be used
OK Vendor tag should not be used
OK Distribution tag should not be used
OK use License and not Copyright 
OK Summary tag should not end in a period
OK if possible, replace PreReq with Requires(pre) and/or Requires(post)
OK specfile is legible
OK package successfully compiles and builds on at least x86
OK BuildRequires are proper
OK summary should be a short and concise description of the package
OK description expands upon summary (don't include installation instructions)
OK make sure lines are <= 80 characters
 - they are, where possible
OK specfile written in American English
OK make a -doc sub-package if necessary
OK packages including libraries should exclude static libraries if possible
OK don't use rpath
OK config files should usually be marked with %config(noreplace)
OK GUI apps should contain .desktop files
OK should the package contain a -devel sub-package?
OK use macros appropriately and consistently
OK don't use %makeinstall
OK install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot}
OK locale data handling correct (find_lang)
OK consider using cp -p to preserve timestamps
OK split Requires(pre,post) into two separate lines
OK package should probably not be relocatable
OK package contains code
OK package should own all directories and files
OK there should be no %files duplicates
OK file permissions should be okay; %defattrs should be present
NEEDS_FIX %clean should be present
 - you have ${RPM_BUILD_ROOT} and $RPM_BUILD_ROOT elsewhere
OK %doc files should not affect runtime
OK if it is a web app, it should be in /usr/share/%{name} and *not* /var/www
NEEDS_FIX verify the final provides and requires of the binary RPMs

  - do we need a Requires on eclipse-platform?

  $ rpm -qp --requires
../RPMS/x86_64/eclipse-egit-0.2.2-0.git20070826.fc8.x86_64.rpm  
  /usr/bin/rebuild-gcj-db  
  /usr/bin/rebuild-gcj-db  
  java-1.5.0-gcj >= 1.5.0
  java-1.5.0-gcj >= 1.5.0
  libc.so.6()(64bit)  
  libc.so.6(GLIBC_2.2.5)(64bit)  
  libdl.so.2()(64bit)  
  libgcc_s.so.1()(64bit)  
  libgcc_s.so.1(GCC_3.0)(64bit)  
  libgcj_bc.so.1()(64bit)  
  libm.so.6()(64bit)  
  libpthread.so.0()(64bit)  
  librt.so.1()(64bit)  
  libz.so.1()(64bit)  
  rpmlib(CompressedFileNames) <= 3.0.4-1
  rpmlib(PayloadFilesHavePrefix) <= 4.0-1
  rtld(GNU_HASH) 

  $ rpm -qp --provides
../RPMS/x86_64/eclipse-egit-0.2.2-0.git20070826.fc8.x86_64.rpm
  org.spearce.egit.core_0.2.2.200708311149.jar.so()(64bit)  
  org.spearce.egit.ui_0.2.2.200708311149.jar.so()(64bit)  
  org.spearce.jgit_0.2.2.200708311149.jar.so()(64bit)  
  eclipse-egit = 0.2.2-0.git20070826.fc8

NEEDS_FIX run rpmlint on the binary RPMs

  $ rpmlint ../RPMS/x86_64/eclipse-egit-0.2.2-0.git20070826.fc8.x86_64.rpm 
  W: eclipse-egit no-documentation
   - okay
  W: eclipse-egit incoherent-version-in-changelog 0.2.2-1.fc8
0.2.2-0.git20070826.fc8
   - please fix
  W: eclipse-egit invalid-license EPL GPLv2
   - this is fine ... unless we discover some LGPL stuff

SHOULD items:

OK package should include license text in the package and mark it with %doc
 - fine
? package should build on i386
 - it builds on x86_64 :)
OK package should build in mock
NEEDS_FIX we should probably fill in some of feature.xml such as the licence section
? should there be any user-visible eclipse features other than Team->Share?
  No checkout?  I know you said they were making a new release soon with a
  whole bunch of new features so should we wait until then?  I'm legitimately
  asking, not trying to be snide.  I notice a lot of stuff being spewed to the
  console as well ... do they have a bug tracker upstream?  I guess what I'm
  trying to say is that we shouldn't have it be installed by default in the
  Eclipse group of comps.xml just yet.  What do you think?
? should we split the package into two:  the java git implementation and the
  eclipse plugin?  I guess we can do that in the future if anything else
  starts using the java git implementation

-- 
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, or are watching someone who is.




More information about the package-review mailing list