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 scratch-gui to GitHub Actions #9330

Merged
merged 6 commits into from
Nov 27, 2023
Merged

Migrate scratch-gui to GitHub Actions #9330

merged 6 commits into from
Nov 27, 2023

Conversation

delasare
Copy link
Contributor

@delasare delasare commented Nov 2, 2023

Resolves

https://scratchfoundation.atlassian.net/browse/ENG-11

  • Adds nvmrc
  • Adds github actions workflow file for scratch-gui
  • Adds github actions translations daily job

@delasare delasare marked this pull request as draft November 2, 2023 13:32
- add setup
- cache dependencies and dependency outputs
- add build job
- add unit test job
- add integration test job
- add npm deploy
- add gh pages deploy
@delasare delasare changed the title Gha migration Migrate scratch-gui to GitHub Actions Nov 14, 2023
@delasare delasare marked this pull request as ready for review November 14, 2023 19:57
@@ -14,7 +14,6 @@
"build": "npm run clean && webpack --colors --bail",
"clean": "rimraf ./build && mkdirp build && rimraf ./dist && mkdirp dist",
"deploy": "touch build/.nojekyll && gh-pages -t -d build -m \"[skip ci] Build for $(git log --pretty=format:%H -n1)\"",
"prepare": "husky install",

Choose a reason for hiding this comment

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

Good riddance!

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

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

I noticed a couple small changes that I think are needed to keep consistent with previous behavior, but it's looking good!

if: github.ref_name == 'develop'
- uses: actions/setup-node@v3
with:
cache: "npm"
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this line tells the setup-node action to cache the npm package cache, so the "Cache node_modules" item further down shouldn't be necessary

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, never mind. I see now that you're using those cache items to carry files to the other jobs.

GitHub head ref: ${{ github.head_ref }}
EOF
- name: Install Dependencies
run: npm install
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
run: npm install
run: npm ci

Comment on lines 122 to 123
- name: Run Build
run: npm run build
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: Run Build
run: npm run build
- name: Run Build
env:
NODE_ENV: production
run: npm run build

Comment on lines +175 to +176
echo Installed chromium version: ${{ steps.setup-chrome.outputs.chrome-version }}
${{ steps.setup-chrome.outputs.chrome-path }} --version
Copy link
Contributor

Choose a reason for hiding this comment

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

😎

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

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

👍 🚀

@delasare delasare merged commit e097b65 into develop Nov 27, 2023
6 checks passed
@cwillisf cwillisf deleted the gha-migration branch January 9, 2024 19:06
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