-
Notifications
You must be signed in to change notification settings - Fork 30
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
Migrate AR to Github Actions #8886
Conversation
Co-authored-by: George B <705427+georgeblahblah@users.noreply.github.com>
Also, increase `buildNumberOffset` to 27000. Co-authored-by: George B <705427+georgeblahblah@users.noreply.github.com>
Co-authored-by: George B <705427+georgeblahblah@users.noreply.github.com>
12a2586
to
5d2aad7
Compare
Size Change: 0 B 🆕 Total Size: 0 B |
`apps-rendering/dist/assets/` contains `apps-rendering/dist/assets/manifest.json` and `apps-rendering/dist/assets/fonts` so no need to refer to those.
ar-ci.sh
script to run AR build in Github ActionsPrevious structure when unzipping was mobile-apps-rendering -> dist -> server -> <files>. This was breaking deployment. After this change structure will be the expected mobile-apps-rendering -> <files> Co-authored-by: George B <705427+georgeblahblah@users.noreply.github.com>
yarn copy-manifest | ||
yarn copy-fonts | ||
yarn synth | ||
zip -j dist/server/mobile-apps-rendering.zip dist/server/* |
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.
We are replacing the deprecated node-riffraff-artifact
with actions-riff-raff
. node-riffraff-artifact
was zipping under the hood the contents of dist/server
and producing mobile-apps-rendering.zip
. We can verify this in its config file. We are now adding a command to do the same.
The -j option in the command stores files with the given names only. The j option doesn't store directory information. This will produce structure:
mobile-apps-rendering
├── 713.server.js
├── manifest.json
├── server.js
which is the same as what current main produces.
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.
💎 Great work. A potential future optimisation is using actions cache https://github.com/actions/cache previously used here: https://github.com/guardian/apps-rendering/blob/ec2866f0345e8869de3e45b2de4ab11b2d3e20ec/.github/workflows/nodejs.yml#L18
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.
Looks good, great work @ioannakok! I had two suggestions which I think would be good to apply, but not necessarily required to proceed.
.github/workflows/ar-ci.yml
Outdated
- uses: actions/setup-node@v3 | ||
with: | ||
node-version-file: .nvmrc |
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.
- uses: actions/setup-node@v3 | |
with: | |
node-version-file: .nvmrc | |
- uses: actions/setup-node@v3 | |
with: | |
cache: yarn | |
node-version-file: .nvmrc |
.github/workflows/ar-ci.yml
Outdated
|
||
- name: Install dependencies | ||
run: | | ||
npm i -g yarn@1.x |
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.
I don't think we need this step to install yarn separately!
npm i -g yarn@1.x |
See more: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#about-caching-workflow-dependencies Also, do not install yarn separately. Co-authored-by: George B <705427+georgeblahblah@users.noreply.github.com> Co-authored-by: DanielCliftonGuardian <110032454+DanielCliftonGuardian@users.noreply.github.com>
Yes, sounds good @DanielCliftonGuardian, thanks for the recommendation! For this build I did it via the |
AR CI job was successful https://github.com/guardian/dotcom-rendering/actions/runs/6298931426/job/17098705590 |
Continuous Deployment kicked off and was successful: https://riffraff.gutools.co.uk/deployment/view/7d988a20-a9e8-4f80-ab0c-bd2e25fbd314 |
Merged PR to improve naming #8904 |
Migrating to GHA in #8886 has meant we are waiting on the AR status check to be successful, but it doesn't always run because of the `paths-filter`. This change removes the filter, so the action will always run.
Migrating to GHA in #8886 has meant we are waiting on the AR status check to be successful, but it doesn't always run because of the `paths-filter`. This change removes the filter, so the action will always run.
We migrated to GHA in #8886. The repository has a status check on the AR build workflow, but the workflow doesn't always run because of the `paths-ignore`. This change removes the filter, so the action will always run and the status check will pass if the build is successful.
Closes #8676
What does this change?
ar-ci.yml
GitHub workflownode-riffraff-artifact
withactions-riff-raff
to upload the artifacts in Riff-Raff.Why?
We are moving away from TC
Testing
Deployed to CODE and checked various types of articles:
Build
Deployments
Build produced by GH Actions deployed successfully
Build produced by TeamCity and GH Actions produce the same artefacts
Zip produced by TC and GH Actions has the same structure
Release plan
Mobile::mobile-apps-rendering
apps-rendering/artifact.json
ci-ar.sh
node-riffraff-artifact
frompackage.json
node-riffraff-artifact
dependency