[Bug 234031] Review Request: eclipse-pydev - an Eclipse plugin for working with Python.
bugzilla at redhat.com
bugzilla at redhat.com
Mon Apr 2 03:36:35 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-pydev - an Eclipse plugin for working with Python.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=234031
------- Additional Comments From ifoox at redhat.com 2007-04-01 23:36 EST -------
New files:
(people.redhat.com doesn't seem to respond right now, so I'm uploading these
files elsewhere and will move them together with the other ones once it's back)
http://www.igorfoox.com/misc/fedora/eclipse-pydev-1.3.1-2.src.rpm
http://www.igorfoox.com/misc/fedora/eclipse-pydev.spec
(In reply to comment #6)
> Lines beginning with an X need to be fixed. They're all minor - thanks! :)
>
> MUST:
> * package is named appropriately
> * it is legal for Fedora to distribute this
> * license field matches the actual license.
> * license is open source-compatible.
> - I really wish they packed the license text in their VCS. Can you please
> ask them to do so? I won't hold up the review on it but it would be great
> if it was more prominent than just in their "new in 0.9.8.4" section of
> their website.
I will ask them to include the license.
> * specfile name matches %{name}
> X verify source and patches (md5sum matches upstream, know what the patches do)
> - I can't duplicate the md5sum of the tarball, but the contents match except
> for some timestamps of the generation time
> - can we get some comments for the patches? they could also be re-numbered
> if you feel like it :)
Done.
> - should we file the references to 0.9.7.1 issue upstream?
I've brought this up before, this seems to be their preferred way of doing
things, strange as it is.
> * summary and description fine
> X correct buildroot
> - should be:
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
Done.
> ? %{?dist} isn't used ... should it be?
I see no reason why it shouldn't be. Done.
> X license text included in package and marked with %doc
> - upstream doesn't include license text outside of feature.xml and I don't
> want to mark that as %doc; we're okay here
> * package meets FHS (http://www.pathname.com/fhs/)
> * rpmlint on SRPM (see earlier bug comments)
> * changelog format fine
> * Packager tag not used
> * Vendor tag not used
> * Distribution tag not used
> * License used and not Copyright
> * Summary tag does not end in a period
> * if possible, replace PreReq with Requires(pre) and/or Requires(post)
> X specfile is legible
> - there is an extraneous '#' in the comment about how to generate the tarball
> - the instructions for generating the tarball should have an updated VCS tag
> (I know it says "substitute the correct version number" but get rid of that
> comment and fix the actual tag)
> - speaking of the tarball-generating instructions, can we clean them up a
> bit? Let's drop the "following the Eclipse Releng process" bit
Simplified the whole comment and put the appropriate tag.
> - remove pkg_summary and eclipse_name and just type them in directly
Done.
> * package successfully compiles and builds on at least x86
> X BuildRequires are proper
> - why don't we just have BR: eclipse-pde?
You're right, that would be better. Done.
> X make sure lines are <= 80 characters
> - can we split the commons-codec symlinking line?
> - also, the lines with symlinking in %install are too long
I've split them up, although some are still slightly over 80 chars.
> * specfile written in American English
> * no -doc sub-package necessary
> * no libraries
> * no rpath
> * no config files
> * not a GUI app
> * no -devel sub-package necessary
> * macros used appropriately and consistently
> * no %makeinstall
> * install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot}
> * no locale data
> * no cp usage so no need to worry about -p
> * split Requires(pre,post) into two separate lines
> * package not relocatable
> * package contains code
> * package owns all directories and files
> * no %files duplicates
> * file permissions fine
> ? %defattrs present
> - should there be another '-' after the 'root,root'?
I'm not sure, some other specs I've looked at have four components, some have 3.
So I've added it, since it seems that's what most do.
> * %clean present
> * %doc files do not affect runtime
> * not a web app
> * verify the final provides and requires of the binary RPMs
> $ rpm -q --requires -p eclipse-pydev-1.3.1-1.i386.rpm
> /usr/bin/rebuild-gcj-db
> /usr/bin/rebuild-gcj-db
> commons-codec >= 1.3
> eclipse-platform
> java-1.5.0-gcj >= 1.5.0
> java-1.5.0-gcj >= 1.5.0
> junit >= 3.8.1
> jython >= 2.2
> 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
> python
> rpmlib(CompressedFileNames) <= 3.0.4-1
> rpmlib(PayloadFilesHavePrefix) <= 4.0-1
> rtld(GNU_HASH)
>
> $ rpm -q --provides -p eclipse-pydev-1.3.1-1.i386.rpm
> ast.jar.so
> core.jar.so
> parser.jar.so
> pydev-debug.jar.so
> pydev-jython.jar.so
> pydev.jar.so
> refactoring.jar.so
> eclipse-pydev = 1:1.3.1-1
>
> * run rpmlint on the binary RPMs
> - see previous bug comments
>
> SHOULD:
> X package should include license text in the package and mark it with %doc
> * package should build on i386
> ? package should build in mock
> - I haven't tried, but I don't think it'll be a problem
(In reply to comment #7)
> I did a test build of the SRPM, if failed, after installing Jython*, the the
> build went fine, maybe Jython should be a build requirement.
You're right, added.
--
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.
More information about the Fedora-package-review
mailing list