[modules-dev] Review request for 6581297
Andreas.Sterbenz at Sun.COM
Tue Sep 11 17:45:08 PDT 2007
Stanley M. Ho wrote:
> This will support the generation of .jam.pack.gz in the jam tool, as
> well as to recognize the .jam file extension in the pack200 tool.
> bugster: http://monaco.sfbay/detail.jsf?cr=6581297
> webrev: http://javaweb.sfbay.sun.com/~stanleyh/webrev/jsr-277/6581297-3/
Some comments below:
. please explain why it is a good idea to change the imports from 2
lines to 11 lines or undo that change
. the tmp file is not deleted anywhere. There seems to be a call to
. it would seem more reliable if the check at line 153 was for (tfname
== null) rather than duplicate the file name checking logic
. which raises an existing issue: the run() method should first set all
the member variables to null (superpackageName, fname, ...). Otherwise, a
2nd call to run() may not behave correctly / accept invalid arguments.
. shouldn't the required name be .jam.pack.gz rather than .pack.gz?
. if we are trying to make assumption based on the file extension, then
that should always be checked. Otherwise someone could assume that
specifying foo.jam.pack as a name will get them a (non-gzipped) pack'ed
JAM file, which it does not. In other words, verify that the name of the
JAM file ends with either .jam, .jar, or .jam.pack.gz and give an error
More information about the modules-dev