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

[Robert Field]

Looks good.
Please push.

Thanks,
Robert



On June 4, 2015 9:24:30 AM ShinyaYoshida <bitterfoxc at gmail.com> wrote:

> Hi Robert,
> Thank you for your review.
>
> I've updated:
> http://cr.openjdk.java.net/~shinyafox/kulla/8081780/webrev.01/
>
> * declare REPL_DOESNOTMATTER_CLASS_NAME in Util
> * replace string literal in Eval
> * In the test, the string literal is still the string literal
>
> Regards,
> shinyafox(Shinya Yoshida)
>
>
> 2015-06-05 0:47 GMT+09:00 Robert Field <robert.field at oracle.com>:
>
> >   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)
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>