-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
Fix cleanup ruby logic #8306
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
Fix cleanup ruby logic #8306
Conversation
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.
Looking good so far but, stepping back, I wonder if this could all be a Bash function we could shell out to run instead? Shelling out to brew shellenv
only takes 0.3s on my system and if we called ruby.sh
directly we could perhaps be even faster.
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.
Nice work on this so far, @maxim-belkin!
65ed5ab
to
2312998
Compare
524f700
to
2ef8032
Compare
Thanks, @MikeMcQuaid. Why did you add |
@maxim-belkin That's the required system version on macOS (that we're checking in |
53c93ea
to
296e99d
Compare
Align the logic in `cleanup_portable_ruby` with that in `ruby.sh`. Co-authored-by: Maxim Belkin <maxim.belkin@gmail.com> Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>
296e99d
to
a6d2989
Compare
Let's see how this goes, thanks for your work on this @maxim-belkin! |
brew style
with your changes locally?brew tests
with your changes locally?Fixes #7743
Align the logic in
cleanup_portable_ruby
with that inruby.sh
.