On Dec 20, 2012, at 10:18 PM, David Holmes wrote:

> webrevs:
> 
> top-level repo: http://cr.openjdk.java.net/~dholmes/8004265/webrev.top/

These comments in Main.gmk don't seem to make sense:

 117 # Note: This double-colon rule is intentional, to support
 118 # custom make file integration.
 119 images:: source-tips demos images-only

Do lines 117 and 118 just need to be deleted?

> 
> The main change is to simply add profiles and profiles-only as top level make 
> targets (similar to images). There is also a change to remove the hardcoded 
> version information (though this may be handled by a separate CR).
> 
> jdk repo: http://cr.openjdk.java.net/~dholmes/8004265/webrev.jdk/

Can't cover the makefiles 100%, Erik would be best to look at some of this, but 
this is what I have so far:

On JarReorder.java, it seems like you have just deleted a warning that someone 
explicitly asked for
a class to be included, and also explicitly asked for that class to be excluded.
If we are changing the tool so that exclusion just silently trumps any 
inclusion request, seems like we
should just do that and delete this message. I'm fine with that, but the 
if(false) seems a bit terse.

Why are some of the makefiles named with a ".txt" suffix? Like 
makefiles/profile-includes.txt?

Overall, I have always been uncomfortable with these detailed exclude/include 
lists when they get
down to listing specific class files, not that your changes are making it any 
worse, but I do see this
as an opportunity to improve things in the long run by capturing the specifics 
of our product shipments.

So no objections from me at this time, but at some point we need Erik to check 
this out.
Unfortunately, everybody on build-infra will be busy for a few weeks trying to 
get the cutover done. :^(

-kto


Reply via email to