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

refactor: cleanup now-unreferenced proto toolchain type #2620

Merged
merged 4 commits into from
Feb 22, 2025

Conversation

alexeagle
Copy link
Contributor

@alexeagle alexeagle commented Feb 20, 2025

Follow-up to #2604, fixes a breaking change in v1.2.0-rc0

Note that this toolchain_type became unused in that PR. We leave behind an alias to make this a non-breaking change.

Verified in a downstream repo that requires the toolchain_type to register pre-built protoc: https://github.com/aspect-build/toolchains_protoc/pull/50/files

Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@aignas
Copy link
Collaborator

aignas commented Feb 21, 2025

I meant something like

deprecation = "Use @bazel_tools//tools:bzl_srcs instead; targets under //docs are internal.",

@alexeagle
Copy link
Contributor Author

Sure - @aignas to move things along faster please feel free to just click the "pencil" icon and make trivial edits like that as your own commits on my PR 😉
I do that for our contributors pretty frequently to spare the round-trip (unless the comment is something they might learn from)

@rickeylev rickeylev enabled auto-merge February 22, 2025 00:13
@rickeylev
Copy link
Collaborator

I added the deprecation attr setting. It's annoying that the toolchain type is under a "private" directory. They should give a public access point for it.

Queued for merge. I'll cherry pick this into the 1.2 release.

@aignas
Copy link
Collaborator

aignas commented Feb 22, 2025

Sure - @aignas to move things along faster please feel free to just click the "pencil" icon and make trivial edits like that as your own commits on my PR 😉 I do that for our contributors pretty frequently to spare the round-trip (unless the comment is something they might learn from)

I do it when I can and when I have my laptop with me. I am not doing it now not because I don't know how to or am unwilling, so no need to point it out. :)

In other words, everybody does as much as they can. :)

@rickeylev rickeylev added this pull request to the merge queue Feb 22, 2025
Merged via the queue into bazelbuild:main with commit f9779ee Feb 22, 2025
3 of 4 checks passed
rickeylev pushed a commit that referenced this pull request Feb 22, 2025
Follow-up to #2604, fixes a breaking change in v1.2.0-rc0

Note that this toolchain_type became unused in that PR. We leave behind
an alias to make this a non-breaking change.

Verified in a downstream repo that requires the toolchain_type to
register pre-built `protoc`:
https://github.com/aspect-build/toolchains_protoc/pull/50/files

---------

Co-authored-by: Richard Levasseur <[email protected]>
(cherry picked from commit f9779ee)
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