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

ShinyaYoshida bitterfoxc at gmail.com
Thu Jun 4 16:24:27 UTC 2015


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


More information about the kulla-dev mailing list