-
Notifications
You must be signed in to change notification settings - Fork 175
Use npm instead of yarn #4837
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
Use npm instead of yarn #4837
Conversation
Unit Test Results241 tests 241 ✅ 24s ⏱️ Results for commit 82549ba. ♻️ This comment has been updated with latest results. |
|
/test |
|
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/21598012046 (with refid (in response to this comment from @tamirkamara) |
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.
Pull request overview
This PR addresses the issue that yarn is no longer included in the base devcontainer image by switching the UI build process from yarn to npm.
Changes:
- Updated UI version from 0.8.24 to 0.8.25 (patch version bump)
- Modified build script to use npm commands instead of yarn
- Removed package-lock.json from .gitignore to allow npm lockfile tracking
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ui/app/package.json | Version bumped to 0.8.25 following patch versioning for bug fix |
| devops/scripts/build_deploy_ui.sh | Replaced yarn install/build commands with npm equivalents |
| CHANGELOG.md | Added entry documenting the switch from yarn to npm |
| .gitignore | Removed package-lock.json ignore rule to track npm lockfile |
marrobi
left a comment
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.
Im good with this if it doesn't break anything.
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.
|
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/21598012046 (with refid (in response to this comment from @tamirkamara) |
|
Right @JC-wk. Do you want to assist? |
|
@copilot fix and test these in line with the PR
|
|
@tamirkamara @JC-wk I've got copilot to do the rest. Needs reviewing/testing, but looks good to me. |
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.
Pull request overview
Copilot reviewed 10 out of 15 changed files in this pull request and generated 2 comments.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/test |
|
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/21600914981 (with refid (in response to this comment from @marrobi) |
|
@tamirkamara @JC-wk tests pass. Suggest we get it merged. |
|
/test |
|
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/21603972539 (with refid (in response to this comment from @tamirkamara) |
|
I will give this a try and report back |
JC-wk
left a comment
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.
All good
|
/test-force-approve Passed: https://github.com/microsoft/AzureTRE/actions/runs/21600914981 |
|
🤖 pr-bot 🤖 ✅ Marking tests as complete (for commit 82549ba) (in response to this comment from @marrobi) |


resolves #4838
What is being addressed
Yarn is no longer a part of the base image we use in the devcontainer.
How is this addressed
p.s. I'm not really a frontend person and this is purely based on ai recommendation.