-
Notifications
You must be signed in to change notification settings - Fork 17
Add step for updating the package-lock overrides in the update-automation workflow #38
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
| # Install dependencies | ||
| cd code-editor-src | ||
| npm install |
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.
npm install is a time-consuming step. Any way we can parallelize this for all targets instead of looping through them one by one?
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.
Implemented this suggestion by extracting the step for updating the package lock dependencies into a different job (running sequentially with the Run Automation Tasks and Handle Failures jobs) which uses the matrix strategy for all targets. Example run on fork for two targets for code-oss version 1.101: https://github.com/vpaiu/code-editor/actions/runs/17071392654.
| echo "Installing required dependencies" | ||
| sudo apt-get update | ||
| sudo apt-get install -y quilt libxml2-utils jq | ||
| sudo apt-get install -y quilt libxml2-utils jq libkrb5-dev libx11-dev libxkbfile-dev |
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.
Why do we need these extra packages for this job if nothing has changed here other than adding an echo statement?
| - name: Setup environment | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y quilt libxml2-utils jq libkrb5-dev libx11-dev libxkbfile-dev |
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.
Same, I do not think we need dev headers for Kerberos and X11 related libraries for the actions below, they are only needed for the actual build.
| ./scripts/prepare-src.sh "${{ matrix.target }}" | ||
| cd code-editor-src | ||
| npm install |
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.
Nit: The npm-cache could be saved after this step so downstream jobs and subsequent builds would not need to re-fetch them.
Description of changes:
update-automationworkflow for updating thepackage-lockoverrides.npm installto sync up thepackage-lock.jsonfiles with the patchedpackage.jsonfiles.package-lock.jsonfiles that differ from their original inthird-party-srcinto the respective package-lock-overrides sub-folder while maintaining the folder structure.Note: This step will be placed after the build and test step in the
update-automationworkflow.Testing:
Tested on my fork for the code-editor-sagemaker-server target for code-oss version 1.101: https://github.com/vpaiu/code-editor/actions/runs/17068271460.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.