Hi Mandy,
Thanks for all the suggestions!
On 07/02/17 20:03, Mandy Chung wrote:
Please find below a new webrev that incorporates your feedback.
http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.03/
I think this sentence is not needed.
216 * <p>A {@code StackWalker} with this {@code SHOW_REFLECT_FRAMES}
option
217 * will show all reflection frames.
I disagree: the text starts by saying:
"By default reflection frames are hidden. The reflection
frames include [...]"
but the sentence at lines 216-217 is the only place where we say what
the option actually does: it shows all reflection frames.
I think it's needed.
Possibly the sentence at lines 216-217 could be moved to become
the first sentence in the doc comment, but I'd rather not change
it now that the CCC has approved the doc changes as in webrev.03.
Basic.java test - the @bug already includes the bug number. line 67-69 can be
removed.
Done.
Minor comments: StackFrame.getClassName() can replace
StackFrame.getDeclaringClass().getName(). Or if you want to compare package
name, StackFrame.getDeclaringClass().getPackageName is an alternative.
Oh - good remark. StackFrame.getClassName() it is.
Nit: what about renaming SimpleObj to ConstructorNewInstance?
SimpleObj::found should probably name as `framesInUnnamedOrReflectionPackage`.
SimpleObj => ConstructorNewInstance
found => testFramesOrReflectionFrames
(frames in the unnamed package are test frames)
I added some comments as well in the accept() method.
Nit: ReflectionFrames.java test - the test is a bit hard maybe SimpleObj can be
renamed to TestHelper and `found` be `result`. Just trying to make the test
easier to read and understand. You may have better idea.
SimpleObj => StackInspector (it calls walk & getCallerClass
to inspect the stack from its constructor)
found => collectedFrames
Also added some comments to better explain the purposes of
StackInspector and StackInspector.Caller.
Here is the new webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.04/
best regards,
-- daniel
Otherwise looks good.
Mandy
