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

tools: Change order of third-party includes #13286

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented May 12, 2020

Towards #13236. Requires #13259.

The new clang-format-9 prefers quote includes before angle includes. To use a different order, we'd have to insert an empty line between them, and we don't want to do that. So, we'll relent and allow the quote includes to appear first.


This change is Reviewable

@jwnimmer-tri
Copy link
Collaborator Author

+@sammy-tri for both reviews per schedule, please.

Note that r_base..r_1 is already reviewed in #13259. Only review r1_..tip in this PR. (I'll rebase the first commit away after that PR merges.)

@jwnimmer-tri jwnimmer-tri added the status: single reviewer ok https://drake.mit.edu/reviewable.html label May 12, 2020
@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review May 12, 2020 16:26
The new clang-format-9 prefers quote includes before angle includes.
To use a different order, we'd have to insert an empty line between
them, and we don't want to do that. So, we'll relent and allow the
quote includes to appear first.
@jwnimmer-tri
Copy link
Collaborator Author

(Rebase is complete.)

Copy link
Contributor

@sammy-tri sammy-tri left a comment

Choose a reason for hiding this comment

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

I think I hate how this looks, but I can't think of a version of mixing quote and angle includes in the same section which I'd like, and the style guide says "accept whatever clang-format enforces" so, eh :lgtm:

Reviewed 38 of 38 files at r2.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignee sammy-tri(platform)

@jwnimmer-tri
Copy link
Collaborator Author

Yeah. Once the deadline pressure is away, I think for #8197 / #8146 and friends we should settle on angles for everything non-drake. But I didn't want to bite that off yet.

@jwnimmer-tri
Copy link
Collaborator Author

Skipping everything / lsan / debug builds for trivial change. Forcing in now.

@jwnimmer-tri jwnimmer-tri merged commit 8e36e2e into RobotLocomotion:master May 12, 2020
@jwnimmer-tri jwnimmer-tri deleted the clang-format-includes-quote-angle branch May 12, 2020 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high status: single reviewer ok https://drake.mit.edu/reviewable.html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants