> On Oct 22, 2016, at 3:06 AM, Alan Bateman <alan.bate...@oracle.com> wrote: > > On 21/10/2016 22:07, Alexandre (Shura) Iline wrote: > >> Alan, >> >> This is a newer version: >> http://cr.openjdk.java.net/~shurailine/8159523/webrev.03/ >> > The usages looks good in this version, would be interesting to get other > opinions (although this isn't specifically module options, this is really > more about replacing usages of ProcessTools). > > One small comment is that I'm not sure about Task defining ALL-UNNAMED. It > might be simpler to leave addExports taking two options so that the LHS is > more obvious in the tests that use.
Then, would it be more consistent to change addReads(String) should to take source module as the parameter and not the whole string? Where now there is addReads(“module1=module2”) it would not have to be addReads(“module1”, "module2”) where there it is now addReads(“module1=ALL_UNNAMED”) or addReads(“module1”, "ALL_UNNAMED”) there could be addReads(“module1”) If to go this path, addExports(String) should be dropped altogether. One other things which needs to be changed with that is data providers, where they return “module1=module2”, they will need to be returning “module1”, “module2” separately. And that is also positive change IMO. Such example is present in tools/launcher/modules/addreads/AddReadsTest.java test. Shura > > -Alan >
