-
Couldn't load subscription status.
- Fork 51
Use CI toolkit plugin v5 and nvm plugin v0.4.0 #998
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
Conversation
fba3476 to
b2d5565
Compare
| queue: windows | ||
| plugins: [$CI_TOOLKIT_PLUGIN] | ||
| env: | ||
| IS_DEV_BUILD: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why IS_DEV_BUILD was set via $env: before and why it worked when the pipeline should have ignored the $ (see the docs).
Anyway, once I added the bash call, the resolution for the env assignment no longer worked.
Moving the env var definition in the env node seems to have done the trick. I can see in the CI logs that we have "Sentry environment: development" as per:
Line 10 in 98f29e1
| console.log( 'Sentry environment:', isDevEnvironment ? 'development' : 'production' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adjusting that, it seems now it's done in more practical way.
We've added that under #813 to avoid initializing Sentry during CI runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the cleanup.
| queue: windows | ||
| plugins: [$CI_TOOLKIT_PLUGIN] | ||
| env: | ||
| IS_DEV_BUILD: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adjusting that, it seems now it's done in more practical way.
We've added that under #813 to avoid initializing Sentry during CI runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 🧹 🧹 👌
In #998 (comment) I got confirmation that it's okay to use this approach for the env var setting. Given it's already in use, because it would not work otherwise, in the Windows steps, this commit updates all the other steps so we have a consistent setup.

Related issues
Proposed Changes
Testing Instructions
Refer to green Windows CI builds
Pre-merge Checklist