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 ------- Additional Comments From bkonrath@xxxxxxxxxx 2007-09-06 14:09 EST ------- New files: http://bagu.org/eclipse/eclipse-egit.spec http://bagu.org/eclipse/eclipse-egit-0.2.2-0.git20070826.fc8.src.rpm (In reply to comment #1) > 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. The tests are LGPL but we're not shipping them. Should I add LGPL to the License line because it's included in the src.rpm? > 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? No, wget doesn't work with the git web repo. I manually clicked on the link to get the file. > 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 Fixed. > 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? Yes, it should have that. Fixed. > $ 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 Fixed. > 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 I added the information that I could. This patch really needs to be upstream so that this information can be filled in properly. > ? 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. IMO this plugin needs a lot of work to be functional. I asked one of the developers about their timeline but haven't received a reply yet. > I notice a lot of stuff being spewed to the > console as well ... do they have a bug tracker upstream? No, not that I know of. > 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? That seems reasonable. > ? 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 Yeah, I think it should be kept together until something needs it. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review