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. > > 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? /Magnus
