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

[ShinyaYoshida]

I've pushed.

Regards,
shinyafox(Shinya Yoshida)

2015-06-05 1:35 GMT+09:00 Robert Field <robert.field at oracle.com>:

>   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)
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>>