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

ci: migrate from CircleCI to Github Actions #1836

Merged
merged 36 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
96f6960
chore: remove CCI config
ze-flo Jun 11, 2024
eacdeb7
chore: add lint:ci script
ze-flo Jun 11, 2024
0c3ee46
refactor: get IGardenTheme from src rather than ./dist
ze-flo Jun 11, 2024
e8c3827
ci: setup GHA main workflow
ze-flo Jun 11, 2024
d88f5df
ci: make test dependent on build step
ze-flo Jun 11, 2024
28bdf5c
ci: maintain compatibility w/ @zendeskgarden/scripts
ze-flo Jun 11, 2024
c824a85
test: set test:ci to silent
ze-flo Jun 11, 2024
03cb1a5
chore: tweak + clean up CI config
ze-flo Jun 11, 2024
b7d41bb
Revert "chore: remove CCI config"
ze-flo Jun 11, 2024
d865963
ci: add temp CCI config for transition
ze-flo Jun 13, 2024
6b17681
fix: trigger workflow on pull_request open, sync, reopened
ze-flo Jun 14, 2024
a149e1a
chore: remove redundant conditional
ze-flo Jun 14, 2024
f2c080e
chore: use REPO_SLUG to get ower + repo name (WIP)
ze-flo Jun 14, 2024
f566924
ci: smoke-test npm whoami
ze-flo Jun 14, 2024
479f248
chore: PR feedback
ze-flo Jun 17, 2024
f6910c1
ci: restore cci config without deploy
ze-flo Jun 17, 2024
fd80114
chore: PR feedback
ze-flo Jun 18, 2024
40b06f3
ci: drop name fields
ze-flo Jun 18, 2024
7d18b05
ci: check team membership before deploying and publishing (dry-run)
ze-flo Jun 26, 2024
f81f2f3
ci: use correct token
ze-flo Jun 27, 2024
46f344a
ci: restore jobs config
ze-flo Jun 27, 2024
00a4e8a
ci: test with gh-pages actions
ze-flo Jun 28, 2024
b5240e0
trying to re-trigger GHA
ze-flo Jun 28, 2024
b8ce1dd
refactor: only deploy to preview host
ze-flo Jun 28, 2024
ded342c
ci: smoke-test
ze-flo Jun 28, 2024
8c82cc2
deps: bump @zendeskgarden/scripts
ze-flo Jun 28, 2024
09a4dbf
fix: add back required GITHUB_TOKEN
ze-flo Jun 28, 2024
41001b0
ci: fix permissions
ze-flo Jun 28, 2024
abf1547
ci: uncomment
ze-flo Jun 28, 2024
02ffe95
chore: rename deploy jobs + script
ze-flo Jun 28, 2024
eff3aee
Sweep up and enable `publish` job
jzempel Jul 1, 2024
2e07159
Fix circle config
jzempel Jul 1, 2024
d89fbd8
Fix circle config
jzempel Jul 1, 2024
7fd42cc
Add missing CCI executor
jzempel Jul 1, 2024
dbb478c
Fix for default CCI `build` job
jzempel Jul 1, 2024
c480551
Constrain `deploy-staging` to non-main branches
jzempel Jul 1, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 4 additions & 62 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,73 +2,15 @@ version: 2.1
orbs:
node: circleci/[email protected]

references:
executor: &executor
jobs:
check:
executor:
name: node/default
tag: lts

workspace_root: &workspace_root ~/project

attach_workspace: &attach_workspace
attach_workspace:
at: *workspace_root

persist_to_workspace: &persist_to_workspace
persist_to_workspace:
root: *workspace_root
paths: .

jobs:
build:
<<: *executor
steps:
- checkout
- node/install-packages:
override-ci-command: npm ci --ignore-scripts
cache-version: '{{ .Environment.CACHE_VERSION }}'
- run: npm exec -- lerna run build --concurrency=2 # prevent out-of-memory
- *persist_to_workspace

test:
<<: *executor
steps:
- *attach_workspace
- run: npm run test:ci -- --runInBand
- run: '[ $COVERALLS_REPO_TOKEN ] && npm exec -- coveralls < .cache/coverage/lcov.info || true'
- *persist_to_workspace

deploy:
<<: *executor
environment:
NODE_DEBUG: gh-pages
steps:
- *attach_workspace
- run: npm run build:demo
- run: utils/scripts/deploy.mjs

publish:
<<: *executor
steps:
- *attach_workspace
- run: npm set //registry.npmjs.org/:_authToken=$NPM_TOKEN
- run: npm exec -- lerna publish from-git --ignore-scripts --yes
- run: echo "PASSED"

workflows:
main:
jobs:
- build
- test:
requires:
- build
- deploy:
requires:
- test
context: writer
- publish:
requires:
- test
context: maintainer
filters:
branches:
only: main
- check
159 changes: 159 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
name: CI

on: push
Copy link
Member

Choose a reason for hiding this comment

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

All of these CI workflows should only happen on pull_request. That's the typical setting for all CircleCI repos. We just turned it off temporarily on react-components because there was no way to get next to build in addition to main.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this config/GHA now allow us to only build on PR for select branches? 🀞🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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


jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version-file: .nvmrc
cache: npm

- name: Install dependencies
Copy link
Member

Choose a reason for hiding this comment

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

Can we forego name throughout? We're accustomed to seeing commands in CI, and the name tends to hide that info under a section that has to be expanded to view. And similar to CCI, GH Actions that we depend on will provide their own.

Copy link
Contributor Author

@ze-flo ze-flo Jun 18, 2024

Choose a reason for hiding this comment

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

run: npm ci --ignore-scripts

- name: Build
run: npm exec -- lerna run build --concurrency=2

- name: Upload dist artifacts
uses: actions/upload-artifact@v4
with:
name: dist-artifact
path: packages/**/dist
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stores content of packages/**/dist/ for test and publish jobs.

Screenshot 2024-06-11 at 1 56 02β€―PM

To (hopefully πŸ˜…) stay within the 500MB storage monthly allowance, we can explicitly set retention-days in the config:

Ex:

Suggested change
- name: Upload dist artifacts
uses: actions/upload-artifact@v4
with:
name: dist-artifact
path: packages/**/dist
- name: Upload dist artifacts
uses: actions/upload-artifact@v4
with:
name: dist-artifact
path: packages/**/dist
retention-days: 7

Or globally define it from the settings page.

Which way do you prefer?

Copy link
Contributor

@geotrev geotrev Jun 14, 2024

Choose a reason for hiding this comment

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

Is this something we had set previously for CCI? I didn't see an equivalent, so I could go either way. The config route does feel more obvious for us to easily grok in the future, but the settings live independent of commit history.

Copy link
Member

Choose a reason for hiding this comment

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

Let's definitely rely on the setting rather than specify here. I want to be able to control this outside the scope of a PR.


build-demo:
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see this folded directly into deploy. We're not getting a ton of value here, and the new workflow could be simplified with:

  • build
    • lint
    • test
    • deploy
      • publish

where (lint, test, deploy) run in parallel. Furthermore, it makes good sense to wait for a successful build before doing the deploy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks great and it's fast! πŸ”₯ πŸš€

Screenshot 2024-06-18 at 11 48 04β€―AM

On main, do we still want to deploy even if test& lint have not passed?

Copy link
Member

Choose a reason for hiding this comment

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

On main, do we still want to deploy even if test& lint have not passed?

Hmm, good Q. No I don't think we do.

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version-file: .nvmrc
cache: npm

- name: Install dependencies
run: npm ci --ignore-scripts

- name: Build Demo
run: npm run build:demo

- name: Upload demo artifacts
uses: actions/upload-artifact@v4
with:
name: demo-artifact
path: ./demo

lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version-file: .nvmrc
cache: npm

- name: Install dependencies
run: npm ci --ignore-scripts

- name: Lint
run: npm run lint:ci

test:
needs: [build]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version-file: .nvmrc
cache: npm

- name: Install dependencies
run: npm ci --ignore-scripts

- name: Download dist artifacts
uses: actions/download-artifact@v4
with:
name: dist-artifact
path: packages

- name: Test
run: npm run test:ci

- name: Coveralls
if: env.COVERALLS_REPO_TOKEN != ''
run: npm exec -- coveralls < .cache/coverage/lcov.info
env:
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}

deploy:
needs: [build-demo, lint, test]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version-file: .nvmrc
cache: npm

- name: Install dependencies
run: npm ci --ignore-scripts

- name: Download demo artifacts
uses: actions/download-artifact@v4
with:
name: demo-artifact
path: ./demo

- name: Deploy
run: utils/scripts/deploy.mjs
Copy link
Member

Choose a reason for hiding this comment

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

Are we good with GitHub Pages deployments on main after June 30? https://docs.github.com/en/pages/quickstart

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some digging, I found no indications that gh-pages used by deploy.mjs will stop working past June 30th.

As a precaution and to future-proof our workflow, we could set 2 deploy jobs:

  1. deploy-branch: runs when github.ref != 'refs/heads/main' and deploys to Netlify using deploy.mjs
  2. deploy-main: runs when github.ref == 'refs/heads/main' and deploys to gh-pages via actions/deploy-pages@v4.

This can be implemented now but at the small cost of extra verbosity, or I can create a follow-up branch to apply the proposed changes only if deploy.mjs fails to run past the Github's deadline.

Any recommendations?

env:
NETLIFY_TOKEN: ${{ secrets.NETLIFY_TOKEN }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
NETLIFY_SITE_ID: ${{ secrets.NETLIFY_SITE_ID }}
# Maintain compatibility with @zendeskgarden/scripts
CIRCLE_PROJECT_USERNAME: zendeskgarden
CIRCLE_PROJECT_REPONAME: react-components
CIRCLECI: true

publish:
needs: [build, lint, test]
runs-on: ubuntu-latest
if: github.ref == 'refs/heads/main'
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0 # Lerna requires the full history, including tags

- name: Setup Node.js
uses: actions/setup-node@v4
with:
node-version-file: .nvmrc
cache: npm
registry-url: https://registry.npmjs.org # Sets the registry in the project level .npmrc

- name: Install dependencies
run: npm ci --ignore-scripts

- name: Download dist artifacts
uses: actions/download-artifact@v4
with:
name: dist-artifact
path: packages

# - name: Publish to npm
# run: npm exec -- lerna publish from-git --ignore-scripts --yes
# env:
# NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"format:package_json": "lerna exec -- prettier-package-json",
"postinstall": "husky",
"lint": "npm run lint:css && npm run lint:js && npm run lint:md",
"lint:ci": "npm run lint && npm run format:all && npm exec tsc",
"lint:css": "stylelint '{packages,utils}/**/*.{js,ts,tsx}'",
"lint:js": "eslint .storybook/ packages/ utils/ --ext js,mjs,ts,tsx --max-warnings 0",
"lint:md": "markdownlint README.md packages/*/README.md packages/*/demo/**/*.mdx",
Expand All @@ -20,7 +21,7 @@
"start": "storybook dev --no-version-updates -p 6006",
"tag": "utils/scripts/tag.mjs",
"test": "jest --config=utils/test/jest.config.js",
"test:ci": "npm run lint && npm run format:all && npm exec tsc && npm test -- --coverage",
"test:ci": "npm test -- --coverage --runInBand --silent",
"test:watch": "npm test -- --watch",
"preversion": "npm run test:ci",
"version": "npm run build && git add -A"
Expand Down
2 changes: 1 addition & 1 deletion utils/build/styled.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import 'styled-components';
import { IGardenTheme } from '../../packages/theming';
import type { IGardenTheme } from '../../packages/theming/src/index';

declare module 'styled-components' {
// eslint-disable-next-line @typescript-eslint/no-empty-interface
Expand Down