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

api: protoxform tool and API reformat. #8309

Merged
merged 4 commits into from
Sep 23, 2019
Merged

Conversation

htuch
Copy link
Member

@htuch htuch commented Sep 20, 2019

This patch introduces a new tool, protoxform, that will be the basis of
the v2 -> v3 migration tooling. It operates as a Python protoc plugin,
within the same framework as protodoc, and provides the ability to
operate on protoc AST input and generate proto output.

As a first step, the tool is applied reflexively on v2, and functions as
a formatting tool. In later patches, this will be added to
check_format/fix_format scripts and CI.

Part of #8082.

Risk level: medium (it's possible that some inadvertent wire changes
occur, if they do, this patch should be rolled back).
Testing: manual inspection of diff, bazel test //test/..., some
grep/diff scripts to ensure we haven't lost any comments.

Signed-off-by: Harvey Tuch htuch@google.com

This patch introduces a new tool, protoxform, that will be the basis of
the v2 -> v3 migration tooling. It operates as a Python protoc plugin,
within the same framework as protodoc, and provides the ability to
operate on protoc AST input and generate proto output.

As a first step, the tool is applied reflexively on v2, and functions as
a formatting tool. In later patches, this will be added to
check_format/fix_format scripts and CI.

Risk level: medium (it's possible that some inadvertent wire changes
  occur, if they do, this patch should be rolled back).
Testing: manual inspection of diff, bazel test //test/..., some
  grep/diff scripts to ensure we haven't lost any comments.

Signed-off-by: Harvey Tuch <htuch@google.com>
@@ -0,0 +1,405 @@
# protoc plugin to map from FileDescriptorProtos to a canonicaly formatted
Copy link
Member Author

Choose a reason for hiding this comment

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

Everything in api/ is generated by protoxform. The actual tooling deltas are in tool/.

@itayd
Copy link
Contributor

itayd commented Sep 20, 2019

/checkowners!

@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: a #8309 (comment) was created by @itayd.

see: more, trace.

Signed-off-by: Harvey Tuch <htuch@google.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

This is very impressive. TBH I mostly just skimmed the automated changes and the script, and just called out thing that I saw. Nothing is a crisis so up to you whether you want to action here or in follow ups.

api/envoy/api/v2/auth/cert.proto Show resolved Hide resolved
api/envoy/api/v2/cluster/circuit_breaker.proto Outdated Show resolved Hide resolved
api/envoy/api/v2/core/base.proto Show resolved Hide resolved
api/envoy/api/v2/core/health_check.proto Show resolved Hide resolved
tools/protoxform/protoxform.py Show resolved Hide resolved
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice!

@htuch htuch merged commit 08b123a into envoyproxy:master Sep 23, 2019
@htuch htuch deleted the proto-copier branch September 23, 2019 18:21
htuch pushed a commit that referenced this pull request Sep 24, 2019
#8309 and #8100 collided

Risk Level: Low (cleanup)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
htuch added a commit to htuch/envoy that referenced this pull request Sep 24, 2019
Via ./api/migration/v3alpha.sh. This picks up the changes since the last
sync, in particular the major reformat in envoyproxy#8309.

Risk level: Low (not used yet).
Testing: bazel build @envoy_api//...

Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch htuch mentioned this pull request Sep 24, 2019
htuch added a commit that referenced this pull request Sep 24, 2019
Via ./api/migration/v3alpha.sh. This picks up the changes since the last
sync, in particular the major reformat in #8309.

Risk level: Low (not used yet).
Testing: bazel build @envoy_api//...

Signed-off-by: Harvey Tuch <htuch@google.com>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Sep 24, 2019
This patch introduces a new tool, protoxform, that will be the basis of
the v2 -> v3 migration tooling. It operates as a Python protoc plugin,
within the same framework as protodoc, and provides the ability to
operate on protoc AST input and generate proto output.

As a first step, the tool is applied reflexively on v2, and functions as
a formatting tool. In later patches, this will be added to
check_format/fix_format scripts and CI.

Part of envoyproxy#8082.

Risk level: medium (it's possible that some inadvertent wire changes
  occur, if they do, this patch should be rolled back).
Testing: manual inspection of diff, bazel test //test/..., some
  grep/diff scripts to ensure we haven't lost any comments.

Signed-off-by: Harvey Tuch <htuch@google.com>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
This patch introduces a new tool, protoxform, that will be the basis of
the v2 -> v3 migration tooling. It operates as a Python protoc plugin,
within the same framework as protodoc, and provides the ability to
operate on protoc AST input and generate proto output.

As a first step, the tool is applied reflexively on v2, and functions as
a formatting tool. In later patches, this will be added to
check_format/fix_format scripts and CI.

Part of envoyproxy#8082.

Risk level: medium (it's possible that some inadvertent wire changes
  occur, if they do, this patch should be rolled back).
Testing: manual inspection of diff, bazel test //test/..., some
  grep/diff scripts to ensure we haven't lost any comments.

Signed-off-by: Harvey Tuch <htuch@google.com>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
envoyproxy#8309 and envoyproxy#8100 collided

Risk Level: Low (cleanup)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
Via ./api/migration/v3alpha.sh. This picks up the changes since the last
sync, in particular the major reformat in envoyproxy#8309.

Risk level: Low (not used yet).
Testing: bazel build @envoy_api//...

Signed-off-by: Harvey Tuch <htuch@google.com>
danzh2010 pushed a commit to danzh2010/envoy that referenced this pull request Oct 4, 2019
This patch introduces a new tool, protoxform, that will be the basis of
the v2 -> v3 migration tooling. It operates as a Python protoc plugin,
within the same framework as protodoc, and provides the ability to
operate on protoc AST input and generate proto output.

As a first step, the tool is applied reflexively on v2, and functions as
a formatting tool. In later patches, this will be added to
check_format/fix_format scripts and CI.

Part of envoyproxy#8082.

Risk level: medium (it's possible that some inadvertent wire changes
  occur, if they do, this patch should be rolled back).
Testing: manual inspection of diff, bazel test //test/..., some
  grep/diff scripts to ensure we haven't lost any comments.

Signed-off-by: Harvey Tuch <htuch@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants