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

Correctly advertise editions support #1637

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mkruskal-google
Copy link

No description provided.

@puellanivis
Copy link
Collaborator

Thank you for the attention, but the github.com/golang/protobuf module is frozen except for essential bug fixes and regressions from one of the more recent versions.

@mkruskal-google
Copy link
Author

I would actually label this a regression/bug fix. AFAICT this plugin should support edition 2023 just fine because it uses the official golang protoc plugin infra, edition 2023 doesn't affect RPC codegen, and it already sets FEATURE_SUPPORTS_EDITIONS by copying SupportedFeatures. It just fails to specify the edition window it supports, so anyone using this plugin will hit protoc errors when they try to upgrade to editions

Copy link
Collaborator

@puellanivis puellanivis left a comment

Choose a reason for hiding this comment

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

A good point. 🤔 I do wonder though, how likely is it that people would still be using this protoc-gen-go rather than the newer one?

But still, also reasonable, and measured.

(Still needs a proper 👍 from someone with dev access.)

@mkruskal-google
Copy link
Author

mkruskal-google commented Aug 8, 2024

FWIW I hit this trying to migrate someone to editions that was using this plugin :). I ended up just switching them to the newer one, but I figured there might be edge cases where that's not as much of an option.

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.

2 participants