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

Install script bug fix: Adding bin folder to usr/local/ when it is missing #2655

Merged
merged 3 commits into from
Mar 15, 2021

Conversation

tiarnann
Copy link
Contributor

@tiarnann tiarnann commented Mar 13, 2021

Short description 📝

The install script can fail to add the tuist executable when the bin folder does not exist inside /usr/local/. I added a command to check if it does not exist and create it.

Checklist ✅

  • The CHANGELOG.md has been updated to reflect the changes. In case of a breaking change, it's been flagged as such.
  • The code architecture and patterns are consistent with the rest of the codebase.
  • The changes have been tested following the documented guidelines.
  • In case the PR introduces changes that affect users, the documentation has been updated.

@tiarnann tiarnann changed the title Install script bug fix: Adding bin folder to usr/local/bin when it is missing Install script bug fix: Adding bin folder to usr/local/ when it is missing Mar 14, 2021
Copy link
Member

@fortmarek fortmarek left a comment

Choose a reason for hiding this comment

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

Good catch @tiarnann! I have just a small nit 🙂

script/install Outdated
@@ -80,6 +80,7 @@ curl -LSs --output /tmp/tuistenv.zip https://storage.googleapis.com/tuist-releas
ohai "Unzipping tuistenv..."
unzip -o /tmp/tuistenv.zip -d /tmp/tuistenv > /dev/null
ohai "Installing tuistenv..."
[ ! -d "/usr/local/bin" ] && execute_sudo mkdir /usr/local/bin/
Copy link
Member

Choose a reason for hiding this comment

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

Nit: maybe using if condition; then ... fi might be more readable (and it's used in the shell script above?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, it makes it more readable. I added that change

@fortmarek fortmarek requested a review from pepicrft March 14, 2021 15:11
@pepicrft
Copy link
Contributor

@all-contributors add @tiarnann for bug

@allcontributors
Copy link
Contributor

@pepibumur

I've put up a pull request to add @tiarnann! 🎉

@pepicrft pepicrft merged commit 8f6b00d into tuist:main Mar 15, 2021
DimaMishchenko added a commit to DimaMishchenko/tuist that referenced this pull request Mar 15, 2021
* main:
  Rebuild frameworks
  Remove unnecessary step
  Add Luis to the core team
  Fix Xcode version
  Fix syntax issue
  Bump gatsby-plugin-mdx from 1.10.0 to 2.0.1 in /next (tuist#2652)
  Plugins: Finalize support for tuist edit (tuist#2642)
  Drop support for Xcode 11.x (tuist#2651)
  docs: add tiarnann as a contributor (tuist#2657)
  Install script bug fix: Adding bin folder to usr/local/ when it is missing (tuist#2655)

# Conflicts:
#	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants