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

Move Suite version to single place? #3335

Closed
tsusanka opened this issue Feb 3, 2021 · 16 comments · Fixed by #3894
Closed

Move Suite version to single place? #3335

tsusanka opened this issue Feb 3, 2021 · 16 comments · Fixed by #3894
Assignees

Comments

@tsusanka
Copy link
Contributor

tsusanka commented Feb 3, 2021

During every release we are bumping the packge versions to format YY.MM.PATCH (see docs). It is a bit annoying that we have to modify the version of each and every packge, see c2448b3. Could we have just a single file where the version is stored? Or extrapolate it somehow to a single variable? I am not sure if it is possible the way yarn works.

We could also write a tool/regex for that but that seems like an overkill.

@slowbackspace
Copy link
Contributor

maybe something like lerna could do the job?
https://github.com/lerna/lerna/blob/main/commands/version/README.md

@tsusanka
Copy link
Contributor Author

tsusanka commented Feb 3, 2021

Yes! ❤️ We just need to figure out how to ignore some of the packges:

lerna version 21.3.1                                    
lerna notice cli v3.22.1
lerna info versioning independent
lerna info Assuming all packages changed

Changes:
 - @trezor/blockchain-link: 1.0.15 => 21.3.1
 - @trezor/components: 1.0.0 => 21.3.1
 - @trezor/integration-tests: 1.0.0 => 21.3.1 (private)
 - @trezor/landing-page: 21.3.0 => 21.3.1 (private)
 - @trezor/news-api: 1.0.0 => 21.3.1 (private)
 - @trezor/rollout: 1.0.5 => 21.3.1
 - @trezor/suite-data: 21.3.0 => 21.3.1
 - @trezor/suite-desktop: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-native: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-storage: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-web-landing: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-web: 21.3.0 => 21.3.1 (private)
 - @trezor/suite: 21.3.0 => 21.3.1 (private)
 - @trezor/translations-manager: 1.0.0 => 21.3.1

? Are you sure you want to create these versions? (ynH) 

@slowbackspace
Copy link
Contributor

slowbackspace commented Feb 4, 2021

I am wondering if we really need to ignore some packages, ones that currently don't follow suite versioning are:
translations-manager, integration/tests, rollout, news-api , components, blockchain-link. But maybe they could and should? The only package that could be used by some 3rd party is blockchain-link, but I am not sure if someone really uses it. And even then, lerna should bump version number only if there were really some changes in the package. So that should work for 3rd parties fine?

hmm but what we certainly need is, after a version bum in eg. trezor/components lerna should automatically bump dep version in all packages (in package.json) that use trezor/components
...just thinking out loud

@tsusanka
Copy link
Contributor Author

tsusanka commented Mar 3, 2021

I have introduced release-it in #3470 which can be used for version bumping but I have to say I hate it. It is very aggressive and will modify releases without prompting. So I will investigate other options.

@tsusanka
Copy link
Contributor Author

tsusanka commented Mar 3, 2021

@keraf @szymonlesisz what do you think about @slowbackspace's suggestion above? That means bumping all packages:

 - @trezor/blockchain-link: 1.0.15 => 21.3.1
 - @trezor/components: 1.0.0 => 21.3.1
 - @trezor/integration-tests: 1.0.0 => 21.3.1 (private)
 - @trezor/landing-page: 21.3.0 => 21.3.1 (private)
 - @trezor/news-api: 1.0.0 => 21.3.1 (private)
 - @trezor/rollout: 1.0.5 => 21.3.1
 - @trezor/suite-data: 21.3.0 => 21.3.1
 - @trezor/suite-desktop: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-native: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-storage: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-web-landing: 21.3.0 => 21.3.1 (private)
 - @trezor/suite-web: 21.3.0 => 21.3.1 (private)
 - @trezor/suite: 21.3.0 => 21.3.1 (private)
 - @trezor/translations-manager: 1.0.0 => 21.3.1

lerna version seems to do the work just fine. We can omit some packages in lerna.json but that would omit them in general for lerna which we do not want to do I guess?

I am not sure myself, on one side it seems superfluous to bump all packages, on the other hand we could use the same argument for suite-data for example. We bump it even if there is no change. What do you think? Anyone strongly against? It would simplify version bumping a lot.

goodhoko added a commit that referenced this issue Jun 8, 2021
Mark components, suite-data and translations-manager as private because
they're not published anywhere and are used within this repo only.
@goodhoko
Copy link
Contributor

goodhoko commented Jun 8, 2021

forme: todo:

goodhoko added a commit that referenced this issue Jun 11, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
goodhoko added a commit that referenced this issue Jun 11, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
goodhoko added a commit that referenced this issue Jun 15, 2021
Mark components, suite-data and translations-manager as private because
they're not published anywhere and are used within this repo only.
goodhoko added a commit that referenced this issue Jun 15, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
@goodhoko goodhoko added the blocked Blocked by external force. Third party inputs required. label Jun 17, 2021
goodhoko added a commit that referenced this issue Jun 17, 2021
After moving trezor-link to this monorepo (newly named as
@trezor/transport) the flow type-checking broke because flow is not
compatible with yarn workspaces we use here.
facebook/flow#5183

In particular, flow fails to resolve packages hoisted to the root
node_modules directory.

Use yarn's nohoist option to prevent hoisting of all transport's
dependencies. This might be more aggressive than needed - some packages
probably can be hoisted without breaking flow. Leave that for a later
optimization as the transport package is awaiting a big refactoring
soon.
goodhoko added a commit that referenced this issue Jun 18, 2021
After moving trezor-link to this monorepo (newly named as
@trezor/transport) the flow type-checking broke because flow is not
compatible with yarn workspaces we use here.
facebook/flow#5183

In particular, flow fails to resolve packages hoisted to the root
node_modules directory.

Use yarn's nohoist option to prevent hoisting of all transport's
dependencies. This might be more aggressive than needed - some packages
probably can be hoisted without breaking flow. Leave that for a later
optimization as the transport package is awaiting a big refactoring
soon.
goodhoko added a commit that referenced this issue Jun 18, 2021
After moving trezor-link to this monorepo (newly named as
@trezor/transport) the flow type-checking broke because flow is not
compatible with yarn workspaces we use here.
facebook/flow#5183

In particular, flow fails to resolve packages hoisted to the root
node_modules directory.

Use yarn's nohoist option to prevent hoisting of all transport's
dependencies. This might be more aggressive than needed - some packages
probably can be hoisted without breaking flow. Leave that for a later
optimization as the transport package is awaiting a big refactoring
soon.
@goodhoko
Copy link
Contributor

As the PR that blocks this will be most probably merged after I leave this will have to be overtaken by someone else.

I implemented this here. The diff-set is complete but one should verify the CI produces valid builds especially with regards to the auto-updater and Suite version exposed within the app.

See the changed documentation to understand what that PR aims to implement.

I'll dare to assign this to @matejkriz who AFAIK works on the blocking PR. cc @alex-jerechinsky

@goodhoko goodhoko assigned matejkriz and unassigned goodhoko Jul 12, 2021
@tsusanka
Copy link
Contributor Author

tsusanka commented Aug 1, 2021

Also wouldn't switch from Independent to Fixed/Locked mode in Lerna do the trick?

@goodhoko
Copy link
Contributor

@tsusanka sorry for taking so long to respond. Unfortunately fixed mode won't help us here. It's (and Lerna in general) based on assumption that the versioning is used to communicate changes in public packages of the monorepo. This means, among other things, that

  • lerna publish only bumps public packages. That's a complete opposite of what we need - we use the versioning to track evolution of the Suite app and we need the few public packages to be versioned independently from the app.
  • Fixed mode only bumps packages that have changed since the last release. However, in current setup we need to bump the version in most packages regardless of the changes because the versions are propagated into the app. (Eg. version of @trezor/suite-desktop package is shown in the desktop builds to the user.) The PR I linked above alleviates us from this by tracking the suite version in a single place (the top-level package.json).

@matejkriz matejkriz removed the blocked Blocked by external force. Third party inputs required. label Aug 10, 2021
matejkriz pushed a commit that referenced this issue Aug 25, 2021
Mark components, suite-data and translations-manager as private because
they're not published anywhere and are used within this repo only.
matejkriz pushed a commit that referenced this issue Aug 25, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
matejkriz pushed a commit that referenced this issue Aug 25, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
matejkriz pushed a commit that referenced this issue Aug 26, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
matejkriz pushed a commit that referenced this issue Aug 27, 2021
Mark components, suite-data and translations-manager as private because
they're not published anywhere and are used within this repo only.
matejkriz pushed a commit that referenced this issue Aug 27, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
matejkriz pushed a commit that referenced this issue Aug 27, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
matejkriz pushed a commit that referenced this issue Aug 27, 2021
Mark components, suite-data and translations-manager as private because
they're not published anywhere and are used within this repo only.
matejkriz pushed a commit that referenced this issue Aug 27, 2021
The suite version is now duplicated across several packages which makes
the release process more complicated and error-prone.

Use constant 1.0.0 version for all private packages and track the Suite
version in the root package.json only.

Edit the build and CI scripts to read the version from there.

Document this new setup.
@matejkriz
Copy link
Member

QA - what to test?
User should see the app version same as before (in Settings > Application > Suite version and in filename of downloaded Suite app). Please check all platforms (win/linux/mac).

@goodhoko
Copy link
Contributor

In addition to what @matejkriz wrote above it would be good to check that the CI produces properly named artifacts - for instance, the app version is part of the name of the released executables.

@tsusanka
Copy link
Contributor Author

cc @vdovhanych for the next release all we need to do is change suiteVersion in packages/suite/package.json 🎉. Great job!

@vdovhanych
Copy link
Member

cc @vdovhanych for the next release all we need to do is change suiteVersion in packages/suite/package.json 🎉. Great job!

@tsusanka saw the pull request today 🥳 very nice feature for us!

@bosomt
Copy link
Contributor

bosomt commented Aug 29, 2021

QA OK

macOS Apple version

obrazek

macOS Intel version

obrazek

Info:

  • Suite version: desktop 21.9.0 (f9ecf81)
  • Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 11_5_1) AppleWebKit/537.36 (KHTML, like Gecko) TrezorSuite/21.9.0 Chrome/89.0.4389.69 Electron/12.0.0 Safari/537.36
  • OS: MacIntel
  • Screen: 1680x1050
  • Device: model T 2.4.2 regular

@STew790
Copy link
Contributor

STew790 commented Aug 31, 2021

QA OK

Windows 10
image

Info:

  • Suite version: desktop 21.9.0 (1f5e833)
  • Browser: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) TrezorSuite/21.9.0 Chrome/89.0.4389.69 Electron/12.0.0 Safari/537.36
  • OS: Win32
  • Screen: 1920x1080
  • Device: model T 2.4.2 regular

@sorooris
Copy link
Contributor

QA OK

image
image
image

Suite: app 21.9.0 b5a686a
OS: Linux Mint 20.02 (Cinnamon 5.0.5)
Device: model T 2.4.2

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.

8 participants