On 2014-02-12 13:24, Magnus Ihse Bursie wrote:
On 12 feb 2014, at 12:58, Alan Bateman <alan.bate...@oracle.com> wrote:
On 12/02/2014 10:42, Erik Joelsson wrote:
Here is another source location cleanup, this time in the corba repo. The 6
classes in com/sun/tools/corba/se/logutil are only used as a build tool during
compilation of the corba repo. These files should be moved to the
corba/make/tools/src directory so that it's not confused with product code.
Bug: https://bugs.openjdk.java.net/browse/JDK-8034769
Webrev: http://cr.openjdk.java.net/~erikj/8034769/webrev.corba.01/
This looks okay to me. Just one comment (it's really a question) is whether
this is consistent with the jdk repository. In the jdk repository then the
location is make/src/classes.
While tools can be somewhat clearer, I think it's good to mimick the jdk. And src
actually says that this is something needing compilation; a "tools" dir could
just hold shellscripts etc. So I'd recommend to go for make/src/classes here as well.
Also, if you don't think it's too much work, Erik, can you move the source code
to the same package structure as the jdk build tools? I.e sun.build.tools or
whatever it is. While I think it is too lon, (sun.buildtools, would have been
enough), consistency is worth more.
I had a feeling you would ask for this, so sure, will fixup the
structure too.
In passing I see the DELETES value in GensrcCorba.gmk is huge and I wonder why
it wasn't split over multiple lines. It makes side-by-side diffs difficult.
Wow! Didn't see that one before! I'm not a fan of a hard limit on 80 chars, but
that... that's just ridiculous. :-)
It should definitely be moved to a variable and be properly split into multiple
lines. And what is it for? Can we generate it instead?
That's a leftover from very early build infra days. I think it was left
that way intentionally to stand out. I agree that we should fix it.
/Erik
/Magnus
