Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run clang-tidy with readability-braces-around-statements #2899

Merged
merged 2 commits into from
Jan 25, 2020

Conversation

greglandrum
Copy link
Member

During the discussion of #2877 we talked about making a code formatting change to add braces for all if, for, etc. statements, including one liners. This is that change.

Along the way clang-tidy made a few additional mods to the code (using the same settings we used before).

I'm ambivalent about the formatting change and don't think it really adds that much, but I'm not opposed, so if the community think it makes sense, I'll go along. @d-b-w and @bp-kelley: you guys asked for this... think it's worth it?

Other operations here:

  • clang-format the results
  • clean up all the parts that clang-tidy-8 broke

clang-format the results
clean up all the parts that clang-tidy-8 broke
@greglandrum greglandrum added Cleanup Code cleanup and refactoring enhancement labels Jan 23, 2020
@greglandrum greglandrum added this to the 2020_03_1 milestone Jan 23, 2020
@d-b-w
Copy link
Contributor

d-b-w commented Jan 24, 2020

I think that using braces around single lines and using nullptr are sufficiently nice that this would be a good change.

Also, there are a couple other clang-tidy fixers that I've found helpful, and so this makes diffs for applying those in the future smaller :)

(I meant this to be an approving review)

@greglandrum
Copy link
Member Author

Ok, I'll go ahead and merge this. The longer we wait the more conflicts we'll see down the road. :-)

@greglandrum greglandrum merged commit d41752d into rdkit:master Jan 25, 2020
@greglandrum greglandrum deleted the dev/another_clang_tidy_run branch January 25, 2020 13:19
greglandrum added a commit to greglandrum/rdkit that referenced this pull request Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup Code cleanup and refactoring enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants