onflow / flow-cli

The Flow CLI is a command-line interface that provides useful utilities for building Flow applications
https://onflow.org
Apache License 2.0
206 stars 66 forks source link

Latest release & `install.sh` does not correspond with version.txt #1048

Closed jribbink closed 1 year ago

jribbink commented 1 year ago

Instructions

Problem

Because of how the release-drafter GH action works, it creates a "draft" release before the PR has been merged & version.txt in the master tree has changed. This means that if you run the install.sh script, you will have a version installed newer than the version.txt file & will effectively be installing an unreleased Flow CLI version.

For instance, right now the latest version as per the latest release in the repo is v1.1.0. However, v1.1.0 has not been released yet - the version.txt update has not been merged to master. If I ran install.sh, however, I would (potentially dangerously) install v1.1.0 when I really should be installing v1.0.2.

Steps to Reproduce

Run install.md right now. You will get the wrong version.

Acceptance Criteria

Maybe we could get release-drafter to actually mark the release as a draft/pre-release or at least not as the latest release. This would solve the issues with install.sh as well as not showing the release at https://github.com/onflow/flow-cli/releases/latest

sideninja commented 1 year ago

When the release is marked as the latest (which in the case of v1.1.0 already was) it's ok to install it using the install script. The version.txt is mostly delayed with updates to wait for Brew to upgrade to the latest version and at that point, all users are reminded about upgrading. The draft releases are not made available with the install script because they don't have build assets. It's also important to have the flexibility of providing different preview release versions to the install scripts because we sometimes require to have a beta release or internal release. So I could see an argument for getting the default value for the latest release from the version.txt but it's equally ok to just use the latest published release. If you still find this valuable you can change to version.txt, but keep in mind your problem statement is not exactly correct, the release it was used during the install you mentioned was an actual manually released version.

jribbink commented 1 year ago

When the release is marked as the latest (which in the case of v1.1.0 already was) it's ok to install it using the install script. The version.txt is mostly delayed with updates to wait for Brew to upgrade to the latest version and at that point, all users are reminded about upgrading. The draft releases are not made available with the install script because they don't have build assets. It's also important to have the flexibility of providing different preview release versions to the install scripts because we sometimes require to have a beta release or internal release. So I could see an argument for getting the default value for the latest release from the version.txt but it's equally ok to just use the latest published release. If you still find this valuable you can change to version.txt, but keep in mind your problem statement is not exactly correct, the release it was used during the install you mentioned was an actual manually released version.

Ok got it, my bad, I didn't realize the release was manually triggered - thanks for filling me in. That definitely changes a bit because I didn't realize that this release could be considered as final and not a draft

My context is from checking for new versions in the VSCode Cadence extension and It's not really like this does anything wrong then. It's just a bit misleading/weird that we say (for windows) "new version v1.0.2 available" then we install v1.1.0, but for MacOS, we say "new version v1.0.2 available" and install v1.0.2. I can't really come up with a major problem that this is going to cause, and technically I could handle this behaviour within the extension itself.

I suppose we can close it this then 👍.

sideninja commented 1 year ago

No, don't be sorry, it's a great point. It can be changed how it works so feel free to do the work you suggested.