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

Improve Bootsnap behaviour #10706

Merged
merged 1 commit into from Feb 26, 2021
Merged

Conversation

MikeMcQuaid
Copy link
Member

  • further refactor nested conditional to make it clearer
  • allow running on Linux while still excluding Apple Silicon
  • only warn on bundle install failures

@MikeMcQuaid MikeMcQuaid added the critical Critical change which should be shipped as soon as possible. label Feb 25, 2021
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

Comment on lines -21 to +19
if homebrew_bootsnap_enabled && development_tools_installed
if homebrew_bootsnap_enabled
Copy link
Member

Choose a reason for hiding this comment

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

I'd still have the development_tools_installed and make that check only in the LoadError case, but it doesn't matter much.

Comment on lines -29 to +27
Homebrew.install_bundler_gems!
Homebrew.install_bundler_gems!(only_warn_on_failure: true)
Copy link
Member

Choose a reason for hiding this comment

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

Looks good, but probably quite noisy if it prints a massive block of warnings on every run?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup but I'm not sure what I see as a real workaround for that (particularly now it's no longer default as-of #10704)

- further refactor nested conditional to make it clearer
- allow running on Linux while still excluding Apple Silicon
- only warn on `bundle install` failures
@MikeMcQuaid MikeMcQuaid merged commit bde8358 into Homebrew:master Feb 26, 2021
@MikeMcQuaid MikeMcQuaid deleted the bootsnap_tweaks branch February 26, 2021 10:48
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Mar 29, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Mar 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants