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

cargo install outside "if cargo installed" block #61

Closed
ClashTheBunny opened this issue May 8, 2021 · 3 comments · Fixed by #63
Closed

cargo install outside "if cargo installed" block #61

ClashTheBunny opened this issue May 8, 2021 · 3 comments · Fixed by #63

Comments

@ClashTheBunny
Copy link

It seems like this should be moved outside of the if statement. The if statement makes sure that cargo is installed, and after the if block, cargo will either have been installed or not needed to be installed. https://github.com/ellie/atuin/blob/7c87624d8af033ff892bd1c21941207bcbfd3735/install.sh#L118

@bl-ue
Copy link
Contributor

bl-ue commented May 8, 2021

Agreed.

It prints Attempting install with cargo, but it only actually installs atuin if cargo wasn't found. 😄

@ClashTheBunny great find, I'd recommend a PR 👍🏻

@ellie
Copy link
Member

ellie commented May 8, 2021

Yuup, you're correct there 🤦‍♀️ Thanks for pointing this out!

If you want to make a PR feel free, otherwise I'll get it sorted shortly :)

ellie added a commit that referenced this issue May 8, 2021
ellie added a commit that referenced this issue May 8, 2021
@ellie ellie closed this as completed in #63 May 8, 2021
ellie added a commit that referenced this issue May 8, 2021
@ClashTheBunny
Copy link
Author

Thanks! I was on my phone, so it would have felt a little arrogant to try ro get it all right, tabs spaces and all.

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 a pull request may close this issue.

3 participants