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

[skip changelog] Remove protobuf workaround on AppVeyor #329

Merged
merged 2 commits into from
Aug 9, 2019

Conversation

zmoog
Copy link
Contributor

@zmoog zmoog commented Aug 7, 2019

We can now switch back to a current version of the protocol since the
PR protocolbuffers/protobuf#5044 has been
merged to master a released.

We can now switch back to a current version of the protocol since the
PR protocolbuffers/protobuf#5044 has been
merged to master a released.
@zmoog
Copy link
Contributor Author

zmoog commented Aug 7, 2019

@rsora the build on AppVeyor looks fine with protobuf v3.9.1 .. what do you think?

Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

LGTM, let's remove the notice and 🚢

@@ -29,7 +29,7 @@ install:
# protobuf tooling needed at test time. We use a very old version of the compiler
# because of this: https://github.com/protocolbuffers/protobuf/issues/3957
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this notice now that we can get latest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we get the latest also on the Docker image used for the unix builds on drone.io? Now it's using the v3.8.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah let's consolidate versions across different platforms 👍

@masci masci changed the title Remove protobuf workaround on AppVeyor [skip changelog] Remove protobuf workaround on AppVeyor Aug 7, 2019
@masci
Copy link
Contributor

masci commented Aug 7, 2019

I've edited PR title to avoid this to end in the changelog (it doesn't affect users)

zmoog pushed a commit that referenced this pull request Aug 9, 2019
Now we can have the same version on both drone.io and AppVeyor, see PR #329 for the details.
zmoog pushed a commit that referenced this pull request Aug 9, 2019
Now we can have the same version on both drone.io and AppVeyor, see PR #329 for the details.
@zmoog zmoog marked this pull request as ready for review August 9, 2019 09:37
@zmoog zmoog merged commit fc780ef into master Aug 9, 2019
@zmoog zmoog deleted the zmoog/remove-protobuf-workaround-on-appveyor branch August 9, 2019 09:40
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