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

New release: 20.1.0 #676

Closed
wants to merge 1 commit into from
Closed

New release: 20.1.0 #676

wants to merge 1 commit into from

Conversation

osa1
Copy link
Member

@osa1 osa1 commented Jun 14, 2022

Update CHANGELOGs and bump version numbers for the new version.

CHANGELOGs describe the new release policy: plugin and library will have the
same version numbers and users should make sure to use the same versions of the
library and the plugin. This simplifies things for both maintainers and users.

This release is backwards compatible with the current releases of the plugin
and the library, but to sync the version numbers a major bump was required for
protobuf. For protoc_plugin we bump the minor version.

The merge commit will be tagged as protobuf-20.1.0 and released on pub.dev.


Ping @sigurdm @devoncarew @kevmoo @natebosch @mraleph

@sigurdm
Copy link
Collaborator

sigurdm commented Jun 14, 2022

I think we should not do a major bump of protobuf if we are non-breaking, that is disruptive to users.
I also think keeping the version numbers in sync will work poorly.

  • Many times we want to publish changes to protoc_plugin but keep the protobuf version. Publishing new unchanged versions is unnecessary churn.

Rather we should rely on the constraint from protoc_plugin to protobuf to make sure that the versions are kept compatible.

protoc_plugin should really be installed as a dev-dependency, not as a dart pub global activated package for several reasons:

  • You don't always want the same version for all projects.
  • You want the constraints to be checked to ensure you are using compatible versions of the two packages
  • You want pub get/dependabot to give you updates of protoc_plugin.

One downside is that protoc_plugin has dependencies that now become your transitive dependencies, and might conflict with your other dependencies.

This requires people to manually create a binstub for running protoc which is unfortunate.
I opened protocolbuffers/protobuf#9953 to fix this, but I think we would have to make the fix ourselves

An alternative approach would be a binary in protoc_plugin that wrote the binstub, and invoked protoc, then you could do:

> dart run protoc_plugin:protoc my.proto

@osa1
Copy link
Member Author

osa1 commented Jun 14, 2022

I was assuming that the recommended workflow is dart pub global activate as we currently recommend in the README.

If adding the plugin as a dev dependency is an option then it becomes easier to pick versions that are compatible.

From a maintainer's point of view, we still have to maintain two changelogs. To document compatible version, we can say that users should either add the plugin as a dev dependency (so that a compatible version will be picked by the build tool), or use the same version of the library as their plugin.

At least for now, maintaining two changelogs it not too bad. As far as I can see we will need just one item in protoc_plugin README.

In other languages that I used in production, build deps specifying runtime deps of the app always caused a lot of headache. In my experience you usually don't want that. That said, we can use it to our advantage here, and I don't think Dart team has plans to change this any time soon (i.e. make build deps completely separate from runtime deps, allow different versions of packages in build deps and runtime deps).

I will submit a new PR with bumping minor versions and updating plugin CHANGELOG.

@osa1 osa1 closed this Jun 14, 2022
@osa1 osa1 deleted the new_release branch June 14, 2022 10:33
@osa1
Copy link
Member Author

osa1 commented Jun 14, 2022

New PR: #677

@sigurdm
Copy link
Collaborator

sigurdm commented Jun 16, 2022

I was assuming that the recommended workflow is dart pub global activate as we currently recommend in the README.

It is the recommended - but I would wish we could change that recommendation. It will require us to formulate a good strategy for running the tool when it is added as a dev-dependency.

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