RFR (L) 8214417: Add space after/before {} in vmTestbase/nsk/jvmti/[A-I] tests

[David Holmes]

I vote for the spaces around { and }

I would write:

int foo() { return 1; }

so would also write:

static jvmtiEvent testEvents[] = { JVMTI_EVENT_THREAD_START, 
JVMTI_EVENT_THREAD_END };

Cheers,
David

On 29/11/2018 8:29 pm, serguei.spitsyn at oracle.com wrote:
> Hi Alex,
> 
> 
> On 11/28/18 18:18, Alex Menkov wrote:
>> Hi Serguei,
>>
>> On 11/28/2018 16:44, serguei.spitsyn at oracle.com wrote:
>>> There are some implicit rules, like unification and consistency.
>>> We want a space or new line after '{' and before '}'.
>>
>> I believe this rule is about braces for code blocks (new line after 
>> "{", "}" on a separate line), but this are array initializers.
> 
> Array initializers are blocs as well.
> It make it a base for unification.
> 
> 
>> And to me for 1-line array initializers spaces do not improve 
>> readability.
> 
> I politely disagree, at least, they improve it for me. :)
> 
> We can do one simple test.
> What suggestion would make the Jc's awk script simpler?
> If yours then I'm out.
> Otherwise, why does it make simpler for script but not for humans?
> 
> Also, we can wait for one more opinion.
> 
> 
>> So to me this looks like the last item from "Whitespace" is applicable 
>> here:
>> <cite>
>> Try not to change whitespace unless it improves readability or 
>> consistency. (Different editors indent differently, and spurious 
>> indentation changes will just make integrations more difficult.)
>> </cite>
> 
> It was Okay to break this rule when we decided to fix spacing over all 
> the tests.
> 
> Thanks,
> Serguei
> 
> 
>> --alex
>>
>>> Why this case needs to have an exception?
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 11/28/18 16:18, Alex Menkov wrote:
>>>> I don't see such rule (I suppose 
>>>> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide is correct 
>>>> link?)
>>>>
>>>> --alex
>>>>
>>>> On 11/28/2018 16:05, serguei.spitsyn at oracle.com wrote:
>>>>> On 11/28/18 15:43, Alex Menkov wrote:
>>>>>> Hi Jc,
>>>>>>
>>>>>> In the JDK-8212771 review thread cleanup for "{}" was requested 
>>>>>> for statements like
>>>>>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/MethodBind/JvmtiTest/JvmtiTest.cpp, 
>>>>>>
>>>>>>
>>>>>> +#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) { 
>>>>>> printf(str); printf("%d\n",res); return res;}
>>>>>>
>>>>>> +#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != 
>>>>>> err) { printf(str); printf("unexpected error %d\n",res); return res;}
>>>>>>
>>>>>> I.e. something like ";}" -> "; }"
>>>>>>
>>>>>>
>>>>>> I don't think changes like
>>>>>>
>>>>>> -static jvmtiEvent testEvents[] = {JVMTI_EVENT_THREAD_START, 
>>>>>> JVMTI_EVENT_THREAD_END};
>>>>>> +static jvmtiEvent testEvents[] = { JVMTI_EVENT_THREAD_START, 
>>>>>> JVMTI_EVENT_THREAD_END };
>>>>>>
>>>>>> are required.
>>>>>
>>>>> It is better to have it - rules are rules.
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>>
>>>>>> --alex
>>>>>>
>>>>>>
>>>>>> On 11/28/2018 11:20, JC Beyler wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> When working on a previous clean-up (JDK-8212771), I was asked to 
>>>>>>> clean-up also spaces around {}.
>>>>>>>
>>>>>>> Here is the first batch out of 2 to fix these cases. Let me know 
>>>>>>> what you think.
>>>>>>>
>>>>>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8214417/webrev.00/
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214417
>>>>>>>
>>>>>>> Thanks for your reviews :-),
>>>>>>> Jc
>>>>>
>>>
>