Code review and main tree inclusion for java-mvn-src.eclass and java-pkg-simple.eclass

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Code review and main tree inclusion for java-mvn-src.eclass and java-pkg-simple.eclass

Andrey Kislyuk-2
Hi everyone,

I would like to commit a package to the main tree whose dependencies
include the freehep-* libraries from the java-experimental overlay.
Those in turn depend on java-mvn-src.eclass, which depends on
java-pkg-simple.eclass, both written by mvg.

So, I'd like to have these committed to the main tree, which means we
need a code review. Please see here:

http://overlays.gentoo.org/proj/java/browser/java-experimental/eclass

... and let's say after 30 days from this email pending resolution of
any complaints mvg and I will push these to main tree.

-ak

Reply | Threaded
Open this post in threaded view
|

Re: Code review and main tree inclusion for java-mvn-src.eclass and java-pkg-simple.eclass

Bugzilla from Martin.vGagern@gmx.net
Christian Faulhammer wrote:
> java-pkg-simple:
> In my eyes, defining all local variables at the beginning of a function
> makes it easier to read.

Sorry, Fauli, i totally missed your mail as it was only addressed to
gentoo-dev, while I've been intently watching gentoo-java...

java-pkg-simple_src_compile() was designed in a modular fashion, with
blocks for compile, javadoc and package which are pretty much
independent from one another. That's the reason why every one of these
blocks has its own local variables, instead of a single set of locals
for the whole function.

Originally I wrote them as independent functions, and they might well be
turned into such independent functions by adding a few function headers
and braces. So if ever the need arises, that's what I'd do. Splitting
locals would increase the footprint of such a change.

From my #gentoo-java log from 2009-01-03:
(11:24:36 UTC) MvG:
java-pkg-simple_src_compile has five clear parts; would it make sense to
place these in separate functions? This would allow more flexibility for
ebuilds that want to use some but not all of these.
(11:26:20 UTC) ali_bush:
MvG:  up to you,  if they are clear, they could alway be separated out later

To sum things up: yes, I could change the local declarations, but I'd
rather keep things as they are, unless you object more vehemently.

Greetings,
 Martin von Gagern (MvG)