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

protobuf@3.1: remove #45042

Closed
wants to merge 3 commits into from
Closed

protobuf@3.1: remove #45042

wants to merge 3 commits into from

Conversation

jonchang
Copy link
Contributor

@jonchang jonchang commented Oct 9, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Remove protobuf 3.1 as it is quite old.

This also removes supersonic as it is unmaintained, and there are no installs in the last 90 days.

This also bumps a revision for ola and migrates it to protobuf@3.6. This currently requires a patch, see OpenLightingProject/ola#1192 from 2017. Protobuf 3.7+ support doesn't come with ola yet (see OpenLightingProject/ola#1558) so we use 3.6 for now.

@fxcoudert fxcoudert closed this in 4b0eeba Oct 10, 2019
@taquitos
Copy link

Is there any reason besides age that this is removed? I ask because this breaks us. We are pinned to this release due to sharing protos between platforms we can't easily update our protos to be compatible with newer releases.
https://github.com/google/science-journal-ios

@fxcoudert
Copy link
Member

The criteria for versioned formulas are quite strict: https://docs.brew.sh/Versions
If you think we made a mistake in assessing this, and the formula meets the criteria, we can revisit this decision.

Otherwise, it seems like the best thing for you would be to maintain your own specific version in a tap: https://docs.brew.sh/How-to-Create-and-Maintain-a-Tap
Homebrew does not commit to maintaining older versions of software over time

@jonchang
Copy link
Contributor Author

As FX said. Moreover, protobuf 3.1 was only included for use by another homebrew formula that couldn’t update. Now that that’s no longer the case, it got removed. See the last paragraph of the versions document.

@taquitos
Copy link

Thanks for the responses!

depends_on "pkg-config" => :build
depends_on "liblo"
depends_on "libmicrohttpd"
depends_on "libusb"
depends_on "numpy@1.16"
depends_on "protobuf@3.1"
depends_on "python@2" # protobuf@3.1 does not support Python 3
Copy link
Contributor

Choose a reason for hiding this comment

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

@jonchang we still have some bugs with Python 3:
Issue: OpenLightingProject/ola#1506
WIP PR: OpenLightingProject/ola#1538

Copy link
Member

Choose a reason for hiding this comment

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

Hopefully you will have a new release with better support soon, then :)

@peternewman
Copy link
Contributor

Remove protobuf 3.1 as it is quite old.

This also bumps a revision for ola and migrates it to protobuf@3.6. This currently requires a patch, see OpenLightingProject/ola#1192 from 2017. Protobuf 3.7+ support doesn't come with ola yet (see OpenLightingProject/ola#1558) so we use 3.6 for now.

Ironically of course this has broken OLA's 0.10 based Mac CI! 😄

@jonchang jonchang deleted the protobuf-3.1 branch October 20, 2019 12:18
@lock lock bot added the outdated PR was locked due to age label Jan 5, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants