RFR: jmod remove
chris.hegarty at oracle.com
Wed May 9 07:31:32 PDT 2012
I finally got back to this. Comments inline...
> 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.
Right, there was an error in my initial change, we only want to generate
a unique name here not actually create the directory. If the directory
is created there could be a problem when doing the move. I encountered
this on some Windows systems.
I retained the logic ( same as createTempDirectory ) to generate the
name, but it could use UUID or similar.
> A few minor comments:
> Library.java defines the mids parameter, not not the dry parameter.
Got it, thanks.
> 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?
I went with ensureNotInConfiguration.
> 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.
I renamed this to deleteTreeUnchecked. I kept deleteTreeUnchecked as it
makes more of an effort to actually delete all the files. Eventually, I
think we should remove deleteTree and replace it with
deleteTreeUnchecked, but this could be done separately as there may be
more discussion around it.
> Shouldn't Files. deleteUnchecked just return any exception thrown by
> delete rather than dropping the useful exception that delete throws?
Reverted to Files.delete and catch the exception.
> 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
More information about the jigsaw-dev