Spec URL: http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs.spec SRPM URL: http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs-1.3.5-1.src.rpm Description: An Eclipse plugin for the FindBugs Java bug detector.
Updated to version 1.3.6: http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs.spec http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs-1.3.6-1.src.rpm
Updated to version 1.3.7: http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs.spec http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs-1.3.7-1.src.rpm
Hi Jerry, This package generally looks okay but it needs to be updated for the new Eclipse plugin packaging guidelines for post-3.4 (we're using the p2 dropins functionality now). When you have time to update it, let me know and I'll do a formal review. Andrew
I have updated it: http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs.spec http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs-1.3.7-2.src.rpm The package isn't set up to make use of pdebuild possible out of the box, and I'm apparently not smart enough to figure out what to do about that, so I'm sticking with the ant build for now. Note that this package depends on the findbugs application itself (bz #464014), which depends on a library named jFormatString (bz #475603). Both of those are still awaiting review.
pdebuild was designed to make it easy for us to build packages where no upstream build mechanism is exposed (or easily re-usable outside of upstream's build environment). If this project provides a usable ant script that works, by all means, use it :) It's better to stick with the way upstream does it. I can review this package and I'll see what I can do about the other packages.
Thanks for the submission. Here's the review. Lines beginning with X need attention; those beginning with * are okay. Other than a few minor things, I think we're good to go once the dependencies are in. * verify the final provides and requires of the binary RPMs X make sure lines are <= 80 characters - please add a line continuation on line 34 to fix this * package successfully compiles and builds * BuildRequires are proper - the BR on rcp is unnecessary if pde is already there - I recommend dropping the gcj bits 'cause the underlying Eclipse RPMs don't have them and they won't make much of a difference for just this plugin * macros fine * 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} * md5sum matches upstream * skim the summary and description for typos, etc. * summary and description good * correct buildroot * %{?dist} used correctly X license text included in package and marked with %doc - since upstream doesn't do this, it's not necessary to force it, but maybe you could ask upstream to do so in the future? X packages meets FHS (http://www.pathname.com/fhs/) - this should probably be in %{_datadir}/eclipse/dropins not %{_libdir}/eclipse/dropins X rpmlint on <this package>.srpm gives no output - this seems odd: findbugs.src:128: E: hardcoded-library-path in ../../lib/findbugs-tools.jar * changelog format okay * Summary tag does not end in a period * no PreReq * specfile is legible * specfile written in American English * no -doc sub-package necessary * not native, so no rpath, static linking, etc. * no config files * not a GUI app * no -devel necessary * install section begins with rm -rf $RPM_BUILD_ROOT or %{buildroot} * no translations so no locale handling * Requires(pre,post) fine but recommend removing gcj bits so will not be necessary * package not relocatable * package contains code * package owns all directories and files * no %files duplicates * file permissions fine * %clean present * %doc files do not affect runtime * not a web app X run rpmlint on the binary RPMs => no output - there are a lot of warnings about non-relative symlinks. You could fix this by making the symlinks to the stuff in /usr/share/java ../../../ (or whatever) instead - there's one warning about a . file: eclipse-findbugs.x86_64: W: hidden-file-or-dir /usr/lib64/eclipse/dropins/findbugs/plugins/edu.umd.cs.findbugs.plugin.eclipse_1.3.7.20081230/.options - is that file necessary? * I verified that it installs and that the findbugs feature is available (after restarting with -clean ... sigh). I look forward to using it!
Thanks for the review, Andrew. I'm excited about getting this into Fedora. > X make sure lines are <= 80 characters > - please add a line continuation on line 34 to fix this When I do that, I get an embedded newline in the definition of %plugin_dir, which causes the symbolic link commands to break. If you know of some way to do this without picking up the embedded newline, please let me know. > * BuildRequires are proper > - the BR on rcp is unnecessary if pde is already there Ah, missed that. Thanks. > - I recommend dropping the gcj bits 'cause the underlying Eclipse RPMs don't > have them and they won't make much of a difference for just this plugin Okay, they're gone. > X license text included in package and marked with %doc > - since upstream doesn't do this, it's not necessary to force it, but maybe > you could ask upstream to do so in the future? Sure, I'll ask. This upstream is funny. My typical interaction with them is to ask a question, which goes unanswered for months. Then I'll remember that I never got an answer, follow up to it on their mailing list asking for responses, and THEN I'll get a response. > X packages meets FHS (http://www.pathname.com/fhs/) > - this should probably be in %{_datadir}/eclipse/dropins not > %{_libdir}/eclipse/dropins I didn't even pick up on the existence of %{_datadir}/eclipse/dropins from the Eclipse guidelines. I'll have to go look at them again. Fixed. > X rpmlint on <this package>.srpm gives no output > - this seems odd: > > findbugs.src:128: E: hardcoded-library-path in ../../lib/findbugs-tools.jar ?? That's from the findbugs SRPM, not the eclipse-findbugs SRPM, right? X run rpmlint on the binary RPMs => no output > - there are a lot of warnings about non-relative symlinks. You could fix this > by making the symlinks to the stuff in /usr/share/java ../../../ (or whatever) > instead Fixed. > - there's one warning about a . file: > > eclipse-findbugs.x86_64: W: hidden-file-or-dir > /usr/lib64/eclipse/dropins/findbugs/plugins/edu.umd.cs.findbugs.plugin.eclipse_1.3.7.20081230/.options > > - is that file necessary? Only at build time. I don't know why it is getting packaged up. I have now removed it from the final binary RPM. New versions are here: http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs.spec http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs-1.3.7-3.fc10.src.rpm
(In reply to comment #7) > Thanks for the review, Andrew. I'm excited about getting this into Fedora. > > > X make sure lines are <= 80 characters > > - please add a line continuation on line 34 to fix this > > When I do that, I get an embedded newline in the definition of %plugin_dir, > which causes the symbolic link commands to break. If you know of some way to do > this without picking up the embedded newline, please let me know. Bah, just leave it > 80 :) > > X license text included in package and marked with %doc > > - since upstream doesn't do this, it's not necessary to force it, but maybe > > you could ask upstream to do so in the future? > > Sure, I'll ask. This upstream is funny. My typical interaction with them is to > ask a question, which goes unanswered for months. Then I'll remember that I > never got an answer, follow up to it on their mailing list asking for responses, > and THEN I'll get a response. Thanks. This isn't a blocker. > > X packages meets FHS (http://www.pathname.com/fhs/) > > - this should probably be in %{_datadir}/eclipse/dropins not > > %{_libdir}/eclipse/dropins > > I didn't even pick up on the existence of %{_datadir}/eclipse/dropins from the > Eclipse guidelines. I'll have to go look at them again. Fixed. Great, thanks. > > X rpmlint on <this package>.srpm gives no output > > - this seems odd: > > > > findbugs.src:128: E: hardcoded-library-path in ../../lib/findbugs-tools.jar > > ?? That's from the findbugs SRPM, not the eclipse-findbugs SRPM, right? :) Sorry. Your new SRPM gives no rpmlint output. > X run rpmlint on the binary RPMs => no output > > > - there are a lot of warnings about non-relative symlinks. You could fix > this > > by making the symlinks to the stuff in /usr/share/java ../../../ (or whatever) > > instead > > Fixed. I still get "dangling-relative-symlink" but I think these are all correctly relative now. Okay, this package is ready to go. Once the dependencies are in, this can go in. I'll mark it accepted but obviously it won't build until findbugs is in. Thanks, Jerry!
New Package CVS Request ======================= Package Name: eclipse-findbugs Short Description: Eclipse plugin for FindBugs Owners: jjames Branches: F-10 InitialCC:
CVS Done
eclipse-findbugs-1.3.7-4.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/eclipse-findbugs-1.3.7-4.fc10
eclipse-findbugs-1.3.7-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.