[Bug 508836] Review Request: colossus - computer implementation of Titan

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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


Jason Tibbitts <tibbs at math.uh.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|177841(FE-NEEDSPONSOR)      |
               Flag|fedora-review?              |fedora-review+




--- Comment #33 from Jason Tibbitts <tibbs at math.uh.edu>  2009-07-12 20:21:18 EDT ---
Well, not only am I lazy, but:
  I have over 40 other reviews in progress.
  I didn't know which of your scratch builds you wanted me to actually review.
Now I know.

rpmlint says:
  colossus.src: W: strange-permission colossus-gen-tarball.sh 0755
I've never understood why rpmlint cares about this.

  colossus.src:172: W: libdir-macro-in-noarch-package (main package) 
   %attr(-,root,root) %{_libdir}/gcj/%{name}
rpmlint doesn't understand BuildArch being conditional.

So both of those are OK.

You should use %{version} in your BuildRoot: somewhere.  I'm not sure why
you've used %{revdate}.  It shouldn't really matter, but you've obviously
started with one of the recommended values so I don't quite understand why
you'd change it.  This is the only thing I see that needs fixing.  It's so
trivial that I'll go ahead and approve this now and you can fix it when you
check in.

The only other thing I can say is that most packages you'll see have the
scriptlets down before the %files list.  Not sure why, but it seems strange to
see them near the front.

* source files match upstream (manually compared).
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.                                                              
* description is OK.                                                          
* dist tag is present.                                                        
X build root.
* license field matches the actual license.                                   
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
  colossus-0-0.1.20090710svn4432.fc12.x86_64.rpm
   colossus.jar.so()(64bit)
   colossus = 0-0.1.20090710svn4432.fc12
   colossus(x86-64) = 0-0.1.20090710svn4432.fc12
  =
   /bin/sh
   coreutils
   java >= 1.6
   java-gcj-compat >= 1.0.31
   jdom
   jpackage-utils
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libgcj_bc.so.1()(64bit)
   libz.so.1()(64bit)

  colossus-javadoc-0-0.1.20090710svn4432.fc12.x86_64.rpm
   colossus-javadoc = 0-0.1.20090710svn4432.fc12
   colossus-javadoc(x86-64) = 0-0.1.20090710svn4432.fc12
  =
   colossus = 0-0.1.20090710svn4432.fc12
   jpackage-utils

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (gcj and icon-cache).
* code, not content.
* %docs are not necessary for the proper functioning of the package.

Java-specific bits:
* no pre-built jars
* single jar, named after the package
* jarfiles are under _javadir.
* javadocs are under _javadocdir.
* ant called properly.
* wrapper script provided.
* gcj called properly.

APPROVED

-- 
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.