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

Merge adjacent string literals #3559

Merged
merged 2 commits into from Dec 1, 2020
Merged

Merge adjacent string literals #3559

merged 2 commits into from Dec 1, 2020

Conversation

maffoo
Copy link
Contributor

@maffoo maffoo commented Dec 1, 2020

As a follow-up to the switch to using black for formatting with increased line length, this merges adjacent string literals that now fit on one line. (Thanks @dabacon for noticing this: #3516 (comment)).

@maffoo maffoo requested a review from a team December 1, 2020 16:31
@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Dec 1, 2020
@95-martin-orion
Copy link
Collaborator

Is this formatting change captured by the fmt-black tag, or should we merge a different tag to pick it up?

@maffoo
Copy link
Contributor Author

maffoo commented Dec 1, 2020

@95-martin-orion, the fmt-black tag only includes the changes to the formatting script, but no actual formatting changes. If you have changes on an in-progress PR where you've modified files that are changed here, you'd get a merge conflict when merging master in the procedure outlined in #3516 (comment). Those conflicts would have to be fixed like any other conflicts when merging master.

@95-martin-orion
Copy link
Collaborator

Ah. I misunderstood this change as an update to the auto-formatting behavior, but I guess that behavior isn't really necessary outside of handling the recent line-width change. SGTM.

@maffoo
Copy link
Contributor Author

maffoo commented Dec 1, 2020

Sorry, I should have clarified that this was just me search-and-replacing things like ' ' and ' f' in string literals, not a change to the formatter itself.

Copy link
Collaborator

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

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

A couple cases look a bit strange at first glance (mostly assert conditions being split to multiple lines) but it's all formatted correctly. LGTM

@maffoo maffoo merged commit 28764be into master Dec 1, 2020
@maffoo maffoo deleted the u/maffoo/adjacent-strings branch December 1, 2020 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants