-
Notifications
You must be signed in to change notification settings - Fork 132
ci: longer timeout, break out lint checks to own yml #382
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
Changes from all commits
6cd9baf
103506a
c296f99
87de083
171b343
21de818
1bba021
6e0c6ec
1ec6815
7ff69c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| name: Lint | ||
|
|
||
| on: pull_request | ||
|
|
||
| jobs: | ||
| lint-check: | ||
| name: Check for ESLint + Prettier Violations | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Check out Git repository | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: node_modules cache | ||
| uses: actions/cache@v2 | ||
| with: | ||
| path: '**/node_modules' | ||
| key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }} | ||
|
|
||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| yarn install --frozen-lockfile --network-timeout 300000 | ||
| - name: Prettier check | ||
| run: | | ||
| yarn run lint:prettier | ||
| - name: Eslint check | ||
| run: | | ||
| yarn run lint:eslint | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,43 @@ | ||
| name: Test | ||
|
|
||
| on: [push, pull_request] | ||
| # https://github.community/t/how-to-trigger-an-action-on-push-or-pull-request-but-not-both/16662 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this config stop tests on non-main branches?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would stop tests on push for non-main branches, only run when they put a PR on main. This is also an important callout - if we change our core branch name again this is another piece to maintain. thoughts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have we changed the core branch name in the past? Is there a reason to think we might do so again?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forgot to respond here! It changed once from master -> main, but I feel like it'd be pretty stable after that? Just the idea that the head branch can change |
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| pull_request: | ||
| branches: | ||
| - main | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Execute full unit test suite | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| node-version: [10.x, 12.x, 14.x] | ||
| os: [macos-10.14, ubuntu-18.04] | ||
| os: [macOS-latest, ubuntu-latest] | ||
| runs-on: ${{ matrix.os }} | ||
|
|
||
| steps: | ||
| - name: Check out Git repository | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: node_modules cache | ||
| uses: actions/cache@v2 | ||
| with: | ||
| path: '**/node_modules' | ||
| key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }} | ||
|
|
||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
|
|
||
| - name: yarn install | ||
| run: | | ||
| yarn install --frozen-lockfile | ||
|
|
||
| - name: prettier check | ||
| run: | | ||
| yarn run lint:prettier | ||
|
|
||
| - name: eslint check | ||
| run: | | ||
| yarn run lint:eslint | ||
|
|
||
| - name: Build and run tests | ||
| run: | | ||
| make test | ||
| - name: Check out Git repository | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: node_modules cache | ||
| uses: actions/cache@v2 | ||
| with: | ||
| path: '**/node_modules' | ||
| key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }} | ||
|
|
||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v1 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| yarn install --frozen-lockfile --network-timeout 300000 | ||
|
|
||
| - name: Build and run tests | ||
| run: | | ||
| make test | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are the changes in this block all whitespace?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so - just my yml linter; let me check which one i'm using |
||
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.
Do we want
ubuntu-latestoverubuntu-18.04?Uh oh!
There was an error while loading. Please reload this page.
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.
ubuntu-latestis 20, not 18 actually. I changed it here since the environment of the linting probably did not matter as much as the actual unit test suite.Do you have a preference between either? The reason I made this change for mac/ubuntu is cuz the Node SDK actions gave me a warning when I was running not the latest version when I was shoring up the tests there