RFR XXS 8133537: clarify position of unlock options in error messages

[Daniel D. Daugherty]

On 8/17/15 2:59 PM, David Holmes wrote:
> On 17/08/2015 11:08 PM, Daniel D. Daugherty wrote:
>> On 8/16/15 8:09 PM, David Holmes wrote:
>>> Hi Dan,
>>>
>>> As much as it pains me to do this to you do we really need two lines
>>> instead of just changing eg:
>>>
>>> must be enabled via -XX:+UnlockDiagnosticVMOptions
>>>
>>> to
>>>
>>> must be enabled by preceding it with -XX:+UnlockDiagnosticVMOptions
>>
>> Thought about doing it this way, but I didn't want to risk
>> running into a test that was looking for the specific existing
>> error message. As it was, I was worried about a 'golden file'
>> style of test, but (so far) my testing hasn't shown that
>> I've run into that particular style of landmine...
>
> Yes there is a risk with any change in output.
>
>> Also, I think having the separate line makes the requirement
>> stand out more.
>
> Yes but in a detrimental way in my opinion. "Gee if they had to 
> document twice that you put the flag first then there must be a lot of 
> people getting it wrong, so obviously there's a usability issue there."

I can't think of anything to say here that's not been said
before so I'm just gonna move on.


>
>> Can I convince you to move forward with the
>> wording change as it is right now?
>
> Given this whole thing has consumed way too much time anyway - yes.

Thanks. I have to crawl through the test results and then I'll
get this one out of my hair.

Dan


>
> Thanks,
> David
>
>> Dan
>>
>>
>>>
>>> ?
>>>
>>> Thanks,
>>> David
>>>
>>> On 15/08/2015 6:53 AM, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> I have a very small code review request to clarify the wording used
>>>> when the following options are specified in the wrong place:
>>>>
>>>>      -XX:+UnlockDiagnosticVMOptions
>>>>      -XX:+UnlockExperimentalVMOptions
>>>>
>>>> Even though this is a trivial change on the surface, we will not be
>>>> following the HotSpot Trivial Change Rules. This means I need two
>>>> reviewers and one must be a (R)eviewer.
>>>>
>>>> See the bug link for examples of the new output.
>>>>
>>>> 8133537: clarify position of unlock options in error messages
>>>> https://bugs.openjdk.java.net/browse/JDK-8133537
>>>>
>>>> Webrev URL:
>>>> http://cr.openjdk.java.net/~dcubed/8133537-webrev/0-jdk9-hs-rt/
>>>>
>>>> Testing: JPRT -testset hotspot is in process
>>>>           Aurora Adhoc Runtime-SVC Nightly testing (will be submitted
>>>> next)
>>>>           (sanity check to make sure new error message line
>>>>            does not break any tests)
>>>>
>>>> Thanks, in advance, for any comments, questions or suggestions.
>>>>
>>>> Dan
>>