-
Notifications
You must be signed in to change notification settings - Fork 197
Add dart=3.7 pragmas to protobuf lib code #1083
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
base: master
Are you sure you want to change the base?
Conversation
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
|
This is not safe to land. This breaks compatibility with older SDKs. It's troubling that we don't have CI set up to catch this. We must either pin to language version environment:
sdk: ^3.10.0 |
|
Why 3.7? Why can't users who can't upgrade to 3.10 use the older version of the library? The formatter in 3.9 doesn't respect the Why do you want to support 3.7 @natebosch? You're right that if we have a min. supported version policy there should be checks to enforce it. I've been maintaining this library for the past 3.5 years and I'm not aware of any such policy. |
They can use the old version, but they can also accidentally get this new one by In other words the |
|
Good point, we should make sure the pubspec is in sync with the What should be the oldest supported SDK version? Do we have a policy in ecosystem packages for the min. supported version? |
|
I don't think we have a policy. The lower bound cannot be lower than the target of the generated code. So I guess it would make sense to make it ^3.10.0. That means older darts won't benefit from the new protobuf improvements, but I think that is acceptable. |
natebosch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should land this without some mechanism to prevent incorrect updates. Maybe it's sufficient to add a CI run on the lowest supported SDK?
With this we can finally have the same formatting in the internal version and the open source version.
This also syncs some simple changes from internal to the open source.
With this the library code is almost identical internally and in the open source. There are still a few changes that need to be synced (enum names, reflection callbacks), which I'll do separately.
cl/832251175 adds the pragmas internally and reformats internal code. Also includes some trivial syncs from this version to the internal.
To avoid bumping SDK dependency from 3.7 to 3.10, we format with 3.7 instead of 3.10. However note that SDK 3.9 cannot be used to format the repo as it has a bug and the formatter in 3.9 does not respect the
@dart=...lines.