Skip to content

Add Protobuf formatting using buf format - #14907

Merged
Eric-Arellano merged 6 commits into
pantsbuild:mainfrom
jyggen:buf-fmt
Mar 25, 2022
Merged

Eric-Arellano merged 6 commits into
pantsbuild:mainfrom
jyggen:buf-fmt

Conversation

@jyggen

@jyggen jyggen commented Mar 25, 2022

Copy link
Copy Markdown
Member

This PR adds support for Protobuf formatting using the newly released buf format. I took the libery of bumping the default version of bufsince buf format requires Buf v1.2.0 and above. Right now there's no check to make sure the user is actually on >v1.2.0, but that shouldn't be too hard to add if needed.

Eric-Arellano
Eric-Arellano previously approved these changes Mar 25, 2022

@Eric-Arellano Eric-Arellano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is super exciting!

Comment thread src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py Outdated
Comment thread src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py Outdated
@Eric-Arellano
Eric-Arellano dismissed their stale review March 25, 2022 05:33

Oops, didn't mean to hit approve. Looks good other than name being ambiguous

@jyggen

jyggen commented Mar 25, 2022 •

Copy link
Copy Markdown
Member Author

Buf added an --exit-code flag to buf format in a recent commit. I'll bump Pants' default Buf version once it's been released and fix the format rule.

@Eric-Arellano Eric-Arellano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great - will approve & merge once the naming weirdness is finalized

Comment thread src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py Outdated
Comment thread src/python/pants/option/option_types.py Outdated
@Eric-Arellano

Copy link
Copy Markdown
Contributor

cc @thejcannon - this will conflict with your very exciting PR at #14903, but I'd like to merge this one first so that we can cherry-pick to 2.11 (and less churn for Jonas)

@Eric-Arellano
Eric-Arellano merged commit 073f12e into pantsbuild:main Mar 25, 2022
jyggen added a commit to jyggen/pants that referenced this pull request Mar 25, 2022
This PR adds support for Protobuf formatting using the newly released `buf format`. I took the libery of bumping the default version of `buf`since `buf format` requires Buf v1.2.0 and above. Right now there's no check to make sure the user is actually on >v1.2.0, but that shouldn't be too hard to add if needed.
Eric-Arellano pushed a commit that referenced this pull request Mar 25, 2022
)

This PR adds support for Protobuf formatting using the newly released `buf format`. I took the libery of bumping the default version of `buf`since `buf format` requires Buf v1.2.0 and above. Right now there's no check to make sure the user is actually on >v1.2.0, but that shouldn't be too hard to add if needed.

[ci skip-rust]
Eric-Arellano pushed a commit that referenced this pull request Mar 26, 2022
Some simple validation to make sure that two or more `LintTargetsRequest` don't use the same name which, based on #14907 (comment), can cause some havoc. I assume this can cause issues in most goals, so might be interesting to add to more places as well.

[ci skip-rust]
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants