RFR: jmod remove
Alan Bateman
Alan.Bateman at oracle.com
Wed May 2 07:23:56 PDT 2012
On 30/04/2012 22:22, Chris Hegarty wrote:
> Thanks Jon and Paul, both very good suggestions.
>
> Updated:
> http://cr.openjdk.java.net/~chegar/jigsaw/jmodrm.04/webrev/
Thanks for adding "rm" as a command and fixing the other issues that
came up in the first round.
I've looked at the latest webrev and it's looks much better.
Paul's suggestion to atomically move the module directory to a trash
location is good suggestion and I see you've named it ".trash". One
comment on moduleTrashDir is that it duplicates the logic for temporary
directories and maybe it would be simper to use use
Files.createTempDirectory to create a directory in the trash directory.
A few minor comments:
Library.java defines the mids parameter, not not the dry parameter.
SimpleLibrary.remove, you can use try-with-resources at L1501.
checkRootsRequire - I wonder if we can find a better name for this
method, the logic seems right and should work for service usage too but
the name of the method seems a bit odd. Maybe
ensureNotPresentInConfiguration or something short?
Just on consistency of naming, then with the change then Files defines
both deleteTree and deleteAllUnchecked to do a recursive delete which
seems a bit inconsistent.
Shouldn't Files. deleteUnchecked just return any exception thrown by
delete rather than dropping the useful exception that delete throws?
Nothing really to do with this patch but ModuleFile.remove(File) could
use Files.readAllLines which would remove much of the code.
Aside from a bit of clean-up then I think this will be great to have
(and urgently needed now because %mids means that we can't rm modules
anymore).
-Alan
More information about the jigsaw-dev
mailing list