Skip to content

thrift 0.12.0#35796

Closed
taraspos wants to merge 3 commits intoHomebrew:masterfrom
taraspos:patch-2
Closed

thrift 0.12.0#35796
taraspos wants to merge 3 commits intoHomebrew:masterfrom
taraspos:patch-2

Conversation

@taraspos
Copy link
Contributor

@taraspos taraspos commented Jan 8, 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>)?

@fxcoudert
Copy link
Member

  • Please squash both commits into a single one, thrift 0.12.0
  • Add to this commit a rename of Aliases/thrift@0.11 to Aliases/thrift@0.12
  • In a second commit, bump revision of osquery

@taraspos
Copy link
Contributor Author

taraspos commented Jan 8, 2019

@fxcoudert
thanks for the feedback, will do.
But I didn't get the one about osquery.
Do you mean update https://github.com/Homebrew/homebrew-core/blob/master/Formula/osquery.rb to 3.3.1?

@taraspos taraspos changed the title Update thrift package to 0.12.0 thrift 0.12.0 Jan 8, 2019
@taraspos
Copy link
Contributor Author

taraspos commented Jan 8, 2019

About the osquery, it is not published on the official downloads page yet https://osquery.io/downloads/official, and I can't find the sha256for the package anywhere else.

@fxcoudert
Copy link
Member

I mean in osquery, bump revision 1 to revision 2, in a separate commit with title osquery: revision for thrift

@taraspos
Copy link
Contributor Author

taraspos commented Jan 8, 2019

All done

@fxcoudert
Copy link
Member

Removing the seldom-used options, per #31510

==> install (30 days)
Index | Name (with options)                                   | Count |  Percent
-----:|-------------------------------------------------------|------:|--------:
1     | thrift                                                | 1,439 |   97.30%
2     | thrift --HEAD                                         |    21 |    1.42%
3     | thrift --with-java                                    |     7 |    0.47%
4     | thrift --with-python@2                                |     7 |    0.47%

@fxcoudert fxcoudert added ready to merge PR can be merged once CI is green and removed ready to merge PR can be merged once CI is green labels Jan 8, 2019
@fxcoudert
Copy link
Member

osquery build failure…

@taraspos
Copy link
Contributor Author

@fxcoudert any ideas what can be done here? 😕

@taraspos taraspos force-pushed the patch-2 branch 2 times, most recently from a1031e3 to 9430210 Compare January 24, 2019 08:57
@SMillerDev SMillerDev added upstream issue An upstream issue report is needed build failure CI fails while building the software labels Jan 24, 2019
@SMillerDev
Copy link
Member

related to #35941

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you removing Java, I think it is important to have the Java bindings. If so, you need to remove the env-settings as well.

Copy link
Member

Choose a reason for hiding this comment

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

Barely anyone uses it based on the analytics above and it pulls in a huge dependency, I'd say this is a valid reason to remove it.

@Fokko Fokko mentioned this pull request Jan 24, 2019
5 tasks
@Fokko
Copy link
Contributor

Fokko commented Feb 4, 2019

Any update on this?

@adamlc
Copy link

adamlc commented Feb 25, 2019

Would love an update if there is one 👍

@linguofeng
Copy link

Any updates?

@taraspos
Copy link
Contributor Author

taraspos commented Mar 1, 2019

Sorry guys, I have no idea what I can do here. The most I can try is to rerun the build, maybe it will work now ¯_(ツ)_/¯.

The upstream issue seems to be ignored.

@fxcoudert
Copy link
Member

fxcoudert commented Mar 4, 2019

The osquery build should be fixed with the new version: 8b42b60

Can you:

  • rebase the thrift update onto master?
  • bump the osquery revision, in a separate commit?

That should do the trick

@fxcoudert fxcoudert removed build failure CI fails while building the software upstream issue An upstream issue report is needed labels Mar 4, 2019
@fxcoudert
Copy link
Member

Thanks @trane9991 ! 🍸

@fxcoudert fxcoudert closed this in ea00488 Mar 4, 2019
@lock lock bot added the outdated PR was locked due to age label Apr 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 3, 2019
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.

6 participants