New webrev: http://cr.openjdk.java.net/~erikj/8034769/webrev.corba.02/
* Moved all the buildtools to the same structure as in the jdk repo.
* Deleted a leftover Makefile that was sitting in the logutil source
directory.
* Moved the idl deletes to a variable with proper line breaks.
/Erik
On 2014-02-12 14:34, Erik Joelsson wrote:
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
