Skip to content
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 giant to github actions #153

Merged
merged 12 commits into from
Oct 2, 2023
Merged

Migrate giant to github actions #153

merged 12 commits into from
Oct 2, 2023

Conversation

philmcmahon
Copy link
Contributor

@philmcmahon philmcmahon commented Oct 2, 2023

What does this change?

Adds a github actions workflow to build giant.

The only change I had to make was to comment out this bit where we manually copy the compiled pdf.worker.min.js file as github actions appears to not have the same issue that teamcity did where it didn't follow the symbolic link associated with the file.

Otherwise this is a fairly standard node/scala gha build

Note that these builds are significantly slower than teamcity- between 13 and 16 minutes up from 7-10 on teamcity. I have emailed DevX asking to use the larger github actions runners.

How to test

I've tested this on playground by deploying a version generated by github actions, it seems to work (including the pdf worker!)

How can we measure success?

@philmcmahon philmcmahon requested a review from a team as a code owner October 2, 2023 10:36
Copy link
Contributor

@marsavar marsavar left a comment

Choose a reason for hiding this comment

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

Looks good!
I think it's worth separating out the frontend and the backend into separate steps rather than having them in the same script; that would allow you to do some interesting stuff with caching (happy to work on it during 10% 😄). But as a first step this seems to work fine.

@@ -21,7 +21,10 @@ popd
cp -r frontend/build/* backend/public
# Replace the symbolic link we use in dev with the actual file.
# On Teamcity the JDeb build doesn't seem to follow the symbolic link while packaging, weirdly
cp frontend/node_modules/pdfjs-dist/build/pdf.worker.min.js backend/public/third-party/pdf.worker.min.js
# NOTE: On Github actions this seems to work, and in fact it will complain if you try and run the below line
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't comment on parts of this file that weren't modified in this PR, but lines 4-11 in this script are made redundant by actions/setup-node@v3 in the workflow so they can be removed I think

# Make Create React App treat warnings as errors
export CI=true
export NVM_DIR="$HOME/.nvm"
[[ -s "$NVM_DIR/nvm.sh" ]] && . "$NVM_DIR/nvm.sh" # This loads nvm
nvm install
nvm use

Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be renamed to ci.sh since it no longer runs in TeamCity.


# Configuring caching is also recommended.
# See https://github.com/actions/setup-java
- name: Setup Java 8
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Setup Java 8
- name: Setup Java 11

@philmcmahon philmcmahon merged commit b9ed569 into main Oct 2, 2023
1 of 2 checks passed
@philmcmahon philmcmahon deleted the pm-gha branch October 2, 2023 13:29
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.

2 participants