Skip to content

Conversation

@ilgooz
Copy link
Member

@ilgooz ilgooz commented May 25, 2021

No description provided.

@ilgooz ilgooz requested a review from barriebyron as a code owner May 25, 2021 08:58
Copy link
Contributor

@barriebyron barriebyron left a comment

Choose a reason for hiding this comment

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

@ilgooz it seems like we can provide some comfort here to existing users with running apps.
Do we want to provide more upgrade guidance?
Users will want to know what happens to their current apps and if they need to make changes? do they need to stop and restart their server? change any settings?

Comment on lines 16 to 20
## Upgrading

Remove any previous Starport installation. To do so, run `rm $(which starport)` command with or without `sudo` depending on your user and repeat until there is no `starport` binary left in your system.

Then follow the regular installation instructions.
Copy link
Contributor

@barriebyron barriebyron May 25, 2021

Choose a reason for hiding this comment

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

Suggested change
## Upgrading
Remove any previous Starport installation. To do so, run `rm $(which starport)` command with or without `sudo` depending on your user and repeat until there is no `starport` binary left in your system.
Then follow the regular installation instructions.
## Upgrading Your Starport Installation
Before you install a new version of Starport, remove all existing Starport installations.
To remove the current Starport installation:
1. Run the `which starport` command to locate the Starport executable file. By default, the Starport binary is installed in `/usr/local/bin`.
1. Change to the directory where Starport is installed. For example:
```bash
cd /usr/local/bin
```
1. Run the remove command. Depending on your user permissions, run the command with or without `sudo`:
```bash
rm starport
```
1. Repeat these steps until all `starport` installations are removed from your system.
After all existing Starport installations are removed, follow the [Installing Starport with cURL](#installing-starport-with-curl) instructions. For details on version features and changes, see the [changelog.md](https://github.com/tendermint/starport/blob/develop/changelog.md) in the repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ilgooz oooh after I uninstalled Starport from the default location, I was surprised to find an unexpected Starport installation:

which starport
/Users/barriebyron/go/bin/starport

Copy link
Contributor

Choose a reason for hiding this comment

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

@ilgooz I think ^^ unexpected installation happened before I knew better than to install go using brew. I had issues with the PATH variables.
Since then, I developed: https://github.com/cosmos/sdk-tutorials/blob/master/TECHNICAL-SETUP.md for onboarding awesomeness.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that usually happens. Different kind of installation options (curl, homebrew, git clone & make install) adds the Starport binary to a different bin folder and users usually forget about them.

Even if you remove a single installation and install a new one, you may end up using another old version of Starport that is still not removed from your system. It is all about the order of presence of the bin dirs in $PATH. This is why I think we should recommend users to uninstall any previous installations.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was surprised to find an unexpected Starport installation:

starport is installed in ~/go/bin when you build it from source, so that's expected.

Copy link
Member Author

@ilgooz ilgooz May 26, 2021

Choose a reason for hiding this comment

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

Two things I would fix from your change requests:

  • /usr/local/bin is not the default bin dir for everyone. It depends on the system. Maybe we should rm this.
  • We don't need two step process for removing the binary by first cd'ing to dir. We can just recommend rm $(which starport) or run which starport first and if any path (binary) found rm xxx it.

@ilgooz ilgooz requested a review from fadeev May 26, 2021 06:12
@ilgooz
Copy link
Member Author

ilgooz commented May 26, 2021

@barriebyron I think we can recommend them to stop chains (stopping serve) before the upgrade.

Migration instructions like changing any configs I think could be described in another page, not necessarily here to keep things simple in the installation docs.

Maybe we can create a migration.md for this purpose later in another PR. In this page we could just show how to migrate things from an older version to the next one when needed.

@ilgooz
Copy link
Member Author

ilgooz commented May 26, 2021

Hey @barriebyron please feel free to add your change requests directly to the PR.

@fadeev
Copy link
Contributor

fadeev commented May 26, 2021

@barriebyron great suggested changes!

Co-authored-by: Barrie Byron <barrie.byron@tendermint.com>
fadeev
fadeev previously approved these changes May 26, 2021
Copy link
Contributor

@fadeev fadeev left a comment

Choose a reason for hiding this comment

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

Looks great!

@barriebyron barriebyron self-requested a review May 26, 2021 17:56
@barriebyron barriebyron requested a review from fadeev May 26, 2021 20:06
barriebyron
barriebyron previously approved these changes May 26, 2021
Copy link
Contributor

@barriebyron barriebyron left a comment

Choose a reason for hiding this comment

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

Ilker, I don't know what the rm $which starport command is, but I trust you. Make sure I got it right.

@ilgooz ilgooz merged commit d841f1f into develop May 27, 2021
@ilgooz ilgooz deleted the docs/upgrade branch May 27, 2021 05:50
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* docs: add upgrading instructions

* Update docs/intro/install.md

Co-authored-by: Barrie Byron <barrie.byron@tendermint.com>

* Ilker review comments, I don't know what the rm  starport command is, but I trust him

* add upgrade highlight to the summary

* Update install.md

Co-authored-by: Denis Fadeev <denis@fadeev.org>
Co-authored-by: Barrie Byron <barrie.byron@tendermint.com>
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.

3 participants