[Bug 433312] Review Request: opengrok - A wicked fast source browser

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

Summary: Review Request: opengrok - A wicked fast source browser
Alias: opengrok-review

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


overholt at redhat.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|overholt at redhat.com         |lkundrak at redhat.com




------- Additional Comments From overholt at redhat.com  2008-04-15 16:08 EST -------
(In reply to comment #10)
> (In reply to comment #9)
> 
> > X specfile is legible
> >   - I'd rather see common_reqs split out and enumerated in both Requires and
> >     BuildRequires
> 
> I thought this will save me from some errors and work when adding common
> requires, but it's just a matter of personal taste. If you both think it doesn't
> look well, I'll split it.

It's not a blocker for us, so if you think it'll be easier to maintain, go for it :)

> > Notes:
> > 
> > - don't build with gcj at all because of this missing bit:
> > 
> > [javac]     import java.util.Scanner;
> > [javac] 	   ^^^^^^^^^^^^^^^^^
> > [javac] The import java.util.Scanner cannot be resolved
> > 
> > - remove gcj bits as the diff I'm attaching does
> 
> Applied. I wonder if it's right that this didn't break the build?

Yeah, it wasn't using libgcj's class library to build before.

> > - re-name patches to match version (0.5 -> 0.6)
> 
> I read somewhere, though I am not able to find the link now, that the version
> number in patch name is one the patch was created against, and doesn't change
> when it applies to newer upstream package.

Okay, if it was generated against 0.5 then keep it.  I guess my question is why
it wasn't applied upstream :)  I like to keep bugs and/or rationale in comments
to denote why we're carrying patches ... but don't worry about it if you don't
want to do it.

> > - why don't you build a jrcs package and Require/BR it?
> 
> AFAIK this is a fork of jrcs from times when it was part of Apache Commons and
> is modified by OpenGrok developers. Currently development of jrcs development
> continues in the place it did before, and I am not aware of any effort to put
> OpenGrok modifications back there.

Okay, well as long as it's building from source (as you have it doing), it's
fine then.

> > - why the big patch between 0.6 and this hg snapshot?  if that's actually
> >   required, why not just use an hg snapshot tarball as SOURCE0 instead of 0.6
> >   and patching?
> 
> About the same reason kernel package does a thing like this. I updated the
> package with new revisions quite rapidly and it would not make much sense to
> waste space with new tarball until patch file has sane length. It can moreover
> be compressed.

Alright, I guess that's acceptable.

> New package:
> 
> http://people.redhat.com/lkundrak/SRPMS/opengrok-0.6-8.hg275.fc8.1.src.rpm
> http://people.redhat.com/lkundrak/SPECS/opengrok.spec

Okay, just two more things:

What's going on with this?

javadoc: error - Error while reading file
/home/overholt/rpmbuild/BUILD/opengrok-0.6-src/jrcs/src/java/org/apache/commons/jrcs/overview.html

And I don't think it should be trying to access the internet using hg during the
build:

-hg-get-changeset:
     [exec] Execute failed: java.io.IOException: Cannot run program "hg":
java.io.IOException: error=2, No such file or directory

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