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

Apply taplo to the repository #18998

Merged
merged 8 commits into from
Jun 8, 2023
Merged

Apply taplo to the repository #18998

merged 8 commits into from
Jun 8, 2023

Conversation

bryanwweber
Copy link
Contributor

Per the suggestion from @thejcannon, I've turned the taplo formatter onto pants itself 😄 Happy to take suggestions for formatting standards, if anyone wants to bikeshed.

Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

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

More tools formatting our files, yay!

@kaos kaos added the category:internal CI, fixes for not-yet-released features, etc. label May 15, 2023
Copy link
Member

@thejcannon thejcannon left a comment

Choose a reason for hiding this comment

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

For the record, here's the available options:
https://taplo.tamasfe.dev/configuration/formatter-options.html

column_width is likely something to bikeshed discuss.

reorder_keys might be interesting to turn on 🤔

pants.ci.toml Outdated Show resolved Hide resolved
@bryanwweber
Copy link
Contributor Author

@thejcannon Is this something we're interested in merging at this time or should I close it?

@thejcannon
Copy link
Member

Oh, yeah totally interested 😄

Mind upgrading the version to one which has the fix from tamasfe/taplo#404?

Copy link
Member

@thejcannon thejcannon left a comment

Choose a reason for hiding this comment

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

We could bikeshed on the specifics, but any amount of auto-formatting is great.

I'll open a thread in #development, and then give it a day, and then we can merge if nothing major comes up

Copy link
Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

I have zero bikeshedding input. I just want there to be a standard, any standard, that is machine-managed. And now we do!

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Yay autoformatters

@cognifloyd
Copy link
Member

I'd like to see line length 100 to match our config for black/isort.

@thejcannon thejcannon enabled auto-merge (squash) June 8, 2023 20:57
@thejcannon
Copy link
Member

I'm enabling auto-merge and will monitor the build.

BIG BIG BIG BIG BIG thanks @bryanwweber ❤️

@bryanwweber
Copy link
Contributor Author

Happy to help <3

@thejcannon thejcannon merged commit e0ef5b8 into pantsbuild:main Jun 8, 2023
@bryanwweber bryanwweber deleted the apply-taplo branch June 8, 2023 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants