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

chore(ci): Nx for github validations #6095

Merged
merged 5 commits into from
Aug 25, 2022
Merged

chore(ci): Nx for github validations #6095

merged 5 commits into from
Aug 25, 2022

Conversation

Nodonisko
Copy link
Contributor

@Nodonisko Nodonisko commented Aug 24, 2022

Nx can performs packages scrips only on smallest possible affected package tree. For example if you will only do changes to @trezor/suite it will check that package and packages that depends on it like suite-desktop. Other packages will be ignored (for example connect packages which doesn't depends on @trezor/suite).

That could speed things a lot and it's will be even better when we will split code into more smaller packages. More smaller packages => faster everything. More info about how Nx works here => https://nx.dev/getting-started/intro

It also has distributed caching that can speed things even more => https://nx.app

This implementation is very limited and we are using only very small subset of Nx features and there is huge space for improvements. Some of the improvements are here #6103

Closes #4936

@socket-security
Copy link

Socket Security Report

Dependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again.

📜 New install scripts detected

A dependency change in this PR is introducing new install scripts to your install step.

Package Script field Location
@parcel/watcher@2.0.4 (added) binding.gyp package.json via nx@14.5.10
nx@14.5.10 (added) postinstall package.json
@parcel/watcher@2.0.4 (added) install package.json via nx@14.5.10
🫣 Native code

Contains native code which could be a vector to obscure malicious code, and generally decrease the likelihood of reproducible or reliable installs.

Package Location
@parcel/watcher@2.0.4 (added) package.json via nx@14.5.10
Socket.dev scan summary
Issue Status
Did you mean? ✅ no new possible package typos
Install scripts ⚠️ 3 new install scripts detected
Telemetry ✅ no new telemetry
Troll package ✅ no new troll packages
Malware ✅ no new malware
Native code ⚠️ 1 new native module detected

Powered by socket.dev

@Nodonisko Nodonisko force-pushed the chore/nx branch 4 times, most recently from 7650e1c to 4a83230 Compare August 24, 2022 14:21
@Nodonisko Nodonisko changed the title WIP: chore(ci): Nx for github validations chore(ci): Nx for github validations Aug 24, 2022
@@ -9,6 +9,7 @@ jobs:
- uses: actions/checkout@v3
with:
submodules: "true"
fetch-depth: 0
Copy link
Member

Choose a reason for hiding this comment

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

Why is depth set to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's recommended in Nx Githib actions guide, without this affected command will throw error because develop branch is not fetched

nx.json Outdated
"lint:styles",
"build:lib"
],
"accessToken": "MzdkNzIwNjUtMWUyZC00YTU2LThlY2YtYjQzYzU0ZThjNTg0fHJlYWQtd3JpdGU=",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe there is a way not to share access token publicly? This way we need to be very sure, that we don't rely on it for release builds.

Copy link
Contributor Author

@Nodonisko Nodonisko Aug 25, 2022

Choose a reason for hiding this comment

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

I think it doesn't matter for this kind of token, it's storing just hashes of packages, not actual build outputs. I would suggest definitely do more investigating before using it any kind of production stuff, but for validation it's good enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will generate read only token that will be here and generate new write token that will be set using env variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old token was invalidated, new one is read-only and second new one with RW rights will be distributed using env variable.

@Nodonisko Nodonisko merged commit a446583 into develop Aug 25, 2022
@Nodonisko Nodonisko deleted the chore/nx branch August 25, 2022 16:51
mroz22 pushed a commit that referenced this pull request Sep 13, 2022
* chore(ci): Nx for github validations

* chore: tune config

* chore: eslint scripts faster and unified settings

* chore: check formatting using nx

* chore: read-only NX token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Consider using Turborepo/Nx
3 participants