Skip to content

bazel: Shift deprecate_* tools to bazel and cleanup requirements#18073

Merged
phlax merged 2 commits intoenvoyproxy:mainfrom
phlax:bazel-deprecate
Sep 16, 2021
Merged

bazel: Shift deprecate_* tools to bazel and cleanup requirements#18073
phlax merged 2 commits intoenvoyproxy:mainfrom
phlax:bazel-deprecate

Conversation

@phlax
Copy link
Copy Markdown
Member

@phlax phlax commented Sep 10, 2021

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: bazel: Shift deprecate_* tools to bazel and cleanup requirements
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #18073 was opened by phlax.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Sep 10, 2021
@phlax phlax changed the title bazel: Shift deprecate_* tools to bazel and cleanup requirements [WI{P] bazel: Shift deprecate_* tools to bazel and cleanup requirements Sep 10, 2021
@phlax phlax changed the title [WI{P] bazel: Shift deprecate_* tools to bazel and cleanup requirements [WIP] bazel: Shift deprecate_* tools to bazel and cleanup requirements Sep 10, 2021
@phlax phlax marked this pull request as draft September 10, 2021 18:58
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Sep 10, 2021

requires repo_path #17293 (now landed)

@phlax phlax force-pushed the bazel-deprecate branch 4 times, most recently from a2ea224 to 62455d1 Compare September 11, 2021 13:13
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax changed the title [WIP] bazel: Shift deprecate_* tools to bazel and cleanup requirements bazel: Shift deprecate_* tools to bazel and cleanup requirements Sep 14, 2021
@phlax phlax marked this pull request as ready for review September 14, 2021 07:27
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Thanks as always for tooling clean up! Do you mind updating deprecate_versions usage in GOVERNANCE.md as well?

@adisuissa @htuch I'm wondering what our plan is for deprecate_features, which previously annotated "deprecated" protos as "fatal-by-default" so we could start cleaning them up. What's the new plan for eventually doing config clean up in envoy and should we remove or replace that script?

@phlax
Copy link
Copy Markdown
Member Author

phlax commented Sep 14, 2021

What's the new plan for eventually doing config clean up in envoy and should we remove or replace that script?

not sure, i was just going through any scripts that didnt use bazel, and moving them. Mostly that means the lesser used ones - some of which may no longer be required

@htuch ?

@phlax phlax changed the title bazel: Shift deprecate_* tools to bazel and cleanup requirements [WIP] bazel: Shift deprecate_* tools to bazel and cleanup requirements Sep 14, 2021
@phlax phlax changed the title [WIP] bazel: Shift deprecate_* tools to bazel and cleanup requirements bazel: Shift deprecate_* tools to bazel and cleanup requirements Sep 14, 2021
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Sep 14, 2021

/wait for GOVERNANCE/docs update

@phlax
Copy link
Copy Markdown
Member Author

phlax commented Sep 14, 2021

/wait

@htuch
Copy link
Copy Markdown
Member

htuch commented Sep 15, 2021

The fatal-by-default for deprecated should continue to be a part of the process. Minor versioning has its own tooling that @adisuissa has built out, which adds in additional annotations on when things are removed in Envoy and there is the 1 year cycle around this. But, this doesn't change the fact that we want to signal early to the community that deprecated features are going away, so +1 to keeping this process around.

@adisuissa
Copy link
Copy Markdown
Contributor

We are still going to have a deprecation period, where deprecated features are still usable, but marked as fatal-by-default. This will serve as a final-warning and encourage upgrade before removing the support.

@alyssawilk
Copy link
Copy Markdown
Contributor

Gotcha. FWIW the script was removed from our release process a while back due to uncertainty around the longer term plan. If we plan on using it it'd be good to make sure it's still WAI and add it back.

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax
Copy link
Copy Markdown
Member Author

phlax commented Sep 16, 2021

ive updated the GOVERNANCE doc

looking through these scripts, if we are going to keep them, we probably want to update them at some point, but this PR at least brings them into the bazel workflow and reduces dep noise

@adisuissa
Copy link
Copy Markdown
Contributor

Gotcha. FWIW the script was removed from our release process a while back due to uncertainty around the longer term plan. If we plan on using it it'd be good to make sure it's still WAI and add it back.

Ack.

Thanks @phlax for the cleanup.

@alyssawilk
Copy link
Copy Markdown
Contributor

/assign-from @envoyproxy/dependency-shepherds

@repokitteh-read-only
Copy link
Copy Markdown

@envoyproxy/dependency-shepherds assignee is @wrowe

🐱

Caused by: a #18073 (comment) was created by @alyssawilk.

see: more, trace.

Copy link
Copy Markdown
Contributor

@wrowe wrowe left a comment

Choose a reason for hiding this comment

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

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Sep 16, 2021
@phlax phlax merged commit f2c69dc into envoyproxy:main Sep 16, 2021
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.

5 participants