-
-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
datafusion 6.0.0 #89562
datafusion 6.0.0 #89562
Conversation
oops i guess in building ballista it requires |
Formula/datafusion.rb
Outdated
@@ -16,8 +16,14 @@ class Datafusion < Formula | |||
end | |||
|
|||
depends_on "rust" => :build | |||
# building ballista requires installing rustfmt | |||
depends_on "rustup-init" => :build |
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.
This seems like the wrong way to go about it. We should instead add a rustfmt
formula and make that a build dependency here.
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.
ok let me try to do that
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.
Thanks. You can open a separate PR for rustfmt
to keep things simple here.
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.
let me try that in #89599
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.
thanks for the advice, this is now updated (and with minimal changes thanks to rustfmt)
@houqp I think we need to (in later releases) update the script so that the tagged commit won't have version including |
Interesting, do you know how it got pulled in? I certainly didn't expect this.
Good call 👍 Will add this to the automation. |
cd8ddcb
to
8203c7b
Compare
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.
Thanks!
🤖 A scheduled task has triggered a merge. |
Created with
brew bump-formula-pr
.