Skip to content

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Mar 3, 2025

Related issues

  • Internal ref pdnsEh-24x-p2

Proposed Changes

  • Use nvm CI plugin v0.4.0 which has clearer logs
  • Use CI toolkit plugin v5.0.0 which comes with more automation for Windows and allows us to remove custom scripts

Testing Instructions

Refer to green Windows CI builds

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors? – N.A.

@mokagio mokagio force-pushed the mokagio/ci-toolkit-v5 branch from fba3476 to b2d5565 Compare March 3, 2025 23:12
@mokagio mokagio changed the title Use CI toolkit plugin v5 and nvm plugin v2 Use CI toolkit plugin v5 and nvm plugin v0.4.0 Mar 3, 2025
queue: windows
plugins: [$CI_TOOLKIT_PLUGIN]
env:
IS_DEV_BUILD: true
Copy link
Contributor Author

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:

console.log( 'Sentry environment:', isDevEnvironment ? 'development' : 'production' );

image

Copy link
Contributor

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.

@mokagio mokagio self-assigned this Mar 3, 2025
@mokagio mokagio requested review from a team and wojtekn March 3, 2025 23:44
@mokagio mokagio marked this pull request as ready for review March 4, 2025 00:10
Copy link
Contributor

@wojtekn wojtekn left a 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
Copy link
Contributor

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.

Copy link
Contributor

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

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

🧹 🧹 🧹 👌

@mokagio mokagio merged commit bc9a0bc into trunk Mar 5, 2025
8 checks passed
@mokagio mokagio deleted the mokagio/ci-toolkit-v5 branch March 5, 2025 04:28
mokagio added a commit that referenced this pull request Mar 5, 2025
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.
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.

4 participants