Skip to content

Specify homebrew node when running homebrew-installed CLI - #753

Merged
amcaplan merged 1 commit into
mainfrom
use-homebrew-node
Nov 9, 2022
Merged

Specify homebrew node when running homebrew-installed CLI#753
amcaplan merged 1 commit into
mainfrom
use-homebrew-node

Conversation

@amcaplan

@amcaplan amcaplan commented Nov 9, 2022

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Fixes #697

Node < 14.17 doesn't work on mac hardware.

WHAT is this pull request doing?

This makes Homebrew-installed CLI use the Homebrew-installed Node formula, which should be a very up-to-date version.

It also has the benefit of decoupling CLI Node from the Node version used by the developer at the time (project settings, system settings, etc.) so nvm use ____ can't break the CLI.

How to test your changes?

vim $(brew --prefix)/bin/shopify and change the first line to #!/usr/bin/env /opt/homebrew/bin/node

Now changing node versions in nvm can't break the CLI!

Post-release steps

This will only take effect once we push the changes to the homebrew-shopify repo.

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.
@amcaplan
amcaplan requested a review from a team November 9, 2022 14:21
@amcaplan
amcaplan merged commit 1f49072 into main Nov 9, 2022
@amcaplan
amcaplan deleted the use-homebrew-node branch November 9, 2022 15:59
FileUtils.move(executable_path, "#{executable_path}-original")
executable_content = <<~SCRIPT
#!/usr/bin/env node
#!/usr/bin/env #{libexec}/bin/node

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hey @amcaplan should this be #!#{libexec}/bin/node? if we hard code the absolute path we shouldn't be needing env anymore

@shopify-shipit
shopify-shipit Bot temporarily deployed to production November 14, 2022 14:38 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants