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

docs(faq): make uninstall instructions shell-independent #2483

Merged
merged 1 commit into from Apr 1, 2021
Merged

docs(faq): make uninstall instructions shell-independent #2483

merged 1 commit into from Apr 1, 2021

Conversation

heyajulia
Copy link
Contributor

@heyajulia heyajulia commented Mar 21, 2021

Description

The uninstall instructions assume that the user is running Bash, which is a safe bet, but isn't always the case.

Motivation and Context

This change wraps the uninstall command in bash -c, making it shell-independent in most cases. It will close issue #2484.

Closes #2484

Screenshots (if appropriate):

N/A

How Has This Been Tested?

I ran bash -c 'rm "$(which starship)"' to uninstall Starship, and it worked, even though I'm using Fish as my shell (this is good).

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@heyajulia heyajulia marked this pull request as ready for review March 21, 2021 15:22
@heyajulia

This comment has been minimized.

@davidkna davidkna changed the title Make uninstall instructions shell-indepedent docs(faq): make uninstall instructions shell-indepedent Mar 21, 2021
@davidkna
Copy link
Member

davidkna commented Mar 21, 2021

I think the commit message is missing the :. docs(faq): make uninstall instructions shell-independent
Also I'd prefer sh over bash.

@davidkna davidkna changed the title docs(faq): make uninstall instructions shell-indepedent docs(faq): make uninstall instructions shell-independent Mar 21, 2021
@heyajulia
Copy link
Contributor Author

heyajulia commented Mar 21, 2021

I think the commit message is missing the :. docs(faq): make uninstall instructions shell-independent

Good catch, thanks!

@chipbuster chipbuster merged commit 4af29d6 into starship:master Apr 1, 2021
@vladimyr
Copy link
Member

vladimyr commented Apr 1, 2021

As I said earlier #2484 (comment) this change seems kinda superfluous. If we really want to go in that direction, we should use sh instead to mirror installation instructions.

@vladimyr
Copy link
Member

vladimyr commented Apr 1, 2021

^ @chipbuster

@chipbuster
Copy link
Contributor

chipbuster commented Apr 1, 2021

Ah, sorry, I think I lost track of the discussion surrounding this PR and thought that that the sh change had already been added. Would it be simplest to just add new PR changing that or do we want to revert this one?

@vladimyr
Copy link
Member

vladimyr commented Apr 1, 2021

Let's add a new one so everybody keeps their contribution! ❤️

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.

Uninstall instructions are specific to Bash
4 participants