Skip to content

Conversation

karel-m
Copy link
Member

@karel-m karel-m commented Jul 3, 2018

This is the last patch related to my clang-tidy testing. After this patch the script .ci/clang-tidy.sh should not throw any warning (at least with clang-tidy 6.0.0).

It is related to braces-around-statements for which the clang-tidy has two plugins: google-readability-braces-around-statements and readability-braces-around-statements.

I have chosen less restrictive google-readability-braces-around-statements.

It allows one line statements without braces:

if (something) some_func(1,2,3);

But does not allow:

if (something)
   some_func(1,2,3);

Which should be rewritten as:

if (something) {
   some_func(1,2,3);
}

@karel-m karel-m requested a review from sjaeckel July 4, 2018 10:34
ocb->checksum[x] ^= 0x80;
else
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I somehow prefer if it's written } else { is that also valid? :)

if yes, can you please change it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@karel-m karel-m force-pushed the pr/clang-tidy-google-readability-braces-around-statements branch from 281d415 to a3dab04 Compare July 6, 2018 11:58
@karel-m karel-m merged commit 98ad88b into develop Jul 6, 2018
@karel-m karel-m deleted the pr/clang-tidy-google-readability-braces-around-statements branch July 6, 2018 15:53
@rofl0r
Copy link

rofl0r commented Jul 6, 2018

advantage of

if (something)
   some_func(1,2,3);

is that you can put a breakpoint on the second line

@karel-m
Copy link
Member Author

karel-m commented Jul 6, 2018

I have changed:

if (something)
   some_func(1,2,3);

into:

if (something) {
   some_func(1,2,3);
}

So this advantage still applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants