xxs: RFR 8081780: JShell tool: Completion contains $REPL00DOESNOTMATTER

[Robert Field]

Hi Shinya,
I don't think DOESNOTMATTER should be part of the API, so making it public 
for tests is not what I would recommend.  The tests can just verify that 
nothing unexpected shows up.  And, yes, it should be defined in Util and 
the constant used in each place including Eval.

Thanks,
Robert



On June 4, 2015 1:16:04 AM ShinyaYoshida <bitterfoxc at gmail.com> wrote:

> Hi Jan,
> Thank you for your review.
>
> Ok, I'll make DOESNOTMATTER as public and fix to reuse it.
>
> I'll push after recieving the responce from Robert.
>
> Regards,
> shinyafox(Shinya Yoshida)
>
>
>
>
> 2015-06-04 16:05 GMT+09:00 Jan Lahoda <jan.lahoda at oracle.com>:
>
> > The changes to completion seem fine to me. I'd suggest to reuse the
> > constant also in tests (no need for another round of review just for that).
> >
> > Thanks!
> >
> > Jan
> >
> >
> > On 3.6.2015 20:18, ShinyaYoshida wrote:
> >
> >> Hi Robert,
> >> Thank you for your review.
> >>
> >> I think the use of "$REPL00DOESNOTMATTER" in Eval#trialCompile is in the
> >> different context between SourceCodeAnalysisImpl.
> >> Appearing $REPL00DOESNOTMATTER in completion is not caused by
> >> Eval#trialCompile but SourceCodeAnalysisImpl#wrapInClass.
> >>
> >> Should I replace "$REPL00DOESNOTMATTER" in Eval#trialCompile by
> >> DOESNOTMATTER even if it is in the different context?
> >>
> >> If I replace, should DOESNOTMATTER be in Util?
> >>
> >> Regards,
> >> shinyafox(Shinya Yoshida)
> >>
> >>
> >> 2015-06-04 2:08 GMT+09:00 Robert Field <robert.field at oracle.com>:
> >>
> >>  Hi Shinya,
> >>>
> >>> Not a full review, but, defining DOESNOTMATTER is good, but then it
> >>> should
> >>> match the code that creates it, so, Eval.trialCompile() should use this
> >>> constant,
> >>>
> >>> Thanks,
> >>> Robert
> >>>
> >>>
> >>> On 06/03/15 09:58, ShinyaYoshida wrote:
> >>>
> >>>  Hi Jan,
> >>>> Please review this.
> >>>>
> >>>> webrev:
> >>>> http://cr.openjdk.java.net/~shinyafox/kulla/8081780/webrev.00/
> >>>>
> >>>> bugs:
> >>>> https://bugs.openjdk.java.net/browse/JDK-8081780
> >>>>
> >>>> Regards,
> >>>> shinyafox(Shinya Yoshida)
> >>>>
> >>>>
> >>>
> >>>