Skip to content

Commit 78d41d0

Browse files
authored
ci: Update frontend file-filters to be each tailored to the tools they relate to (#67251)
Updates the file-filters lists to be more specific for each tool. There are 3 tool categories: linting js (eslint and biome), linting css (stylelint), testing files. Note that each of these 3 categories includes a list of files to check, but also a list of configs that should trigger a run too. So if someone adds a new lint rule to the config we'll re-run everything right away.
1 parent 74bb659 commit 78d41d0

File tree

7 files changed

+115
-84
lines changed

7 files changed

+115
-84
lines changed

.eslintignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,4 @@
22
**/vendor/**/*
33
**/tests/**/fixtures/**/*
44
!.github
5-
!.github/workflows/scripts/*
65
*.d.ts

.github/file-filters.yml

Lines changed: 63 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,69 @@
11
# This is used by the action https://github.com/dorny/paths-filter
22

3-
# TODO: There are some meta files that we potentially could ignore for both front/backend,
4-
# as well as some configuration files that should trigger both
5-
frontend_components_lintable: &frontend_components_lintable
6-
- '**/*.[tj]{s,sx}'
7-
8-
frontend_lintable: &frontend_lintable
9-
- *frontend_components_lintable
10-
- '**/tsconfig*.json'
11-
- '{package,now,vercel}.json'
12-
13-
yarn_lockfile: &yarn_lockfile
14-
- 'yarn.lock'
15-
16-
eslint_config: &eslint_config
17-
- '.eslint*'
18-
19-
# `frontend_src` filters on files that are frontend changes excluding
20-
# changes to the tests/ directory.
21-
# If you want to filter on *all* frontend files, use the `frontend_all` filter.
22-
frontend_src: &frontend_src
23-
- *yarn_lockfile
24-
- *eslint_config
25-
- '!(tests)/**/*.[tj]{s,sx}'
26-
- '**/tsconfig*.json'
27-
- '{package,now,vercel}.json'
28-
- '**/*.less'
29-
- 'static/**'
30-
- 'fixtures/search-syntax/**/*'
31-
- '.github/workflows/frontend.yml'
32-
3+
sentry_specific_workflow: &sentry_specific_workflow
4+
- added|modified: '.github/workflows/frontend.yml'
5+
sentry_specific_test_files: &sentry_specific_test_files
6+
- added|modified: 'tests/js/**/*'
7+
- added|deleted|modified: 'fixtures/search-syntax/**/*'
8+
- added|deleted|modified: 'fixtures/js-stubs/**/*'
9+
10+
# Provides list of changed app files to lint against (stylelint)
11+
lintable_css_in_js_modified: &lintable_css_in_js_modified
12+
- added|modified: '**/*.[tj]{s,sx}'
13+
14+
# Trigger for when we must run full lint (stylelint)
15+
lintable_css_in_js_rules_changed: &lintable_css_in_js_rules_changed
16+
- *sentry_specific_workflow
17+
- added|modified: '.github/file-filters.yml'
18+
- added|modified: 'yarn.lock'
19+
- added|deleted|modified: 'stylelint.*'
20+
21+
# Provides list of changed files to lint against (eslint)
22+
lintable_modified: &lintable_modified
23+
- added|modified: '**/*.[tj]{s,sx}'
24+
- added|modified: '**/*.{html,less,json,yml,md}'
25+
26+
# Trigger for when we must run full lint (eslint)
27+
lintable_rules_changed: &lintable_rules_changed
28+
- *sentry_specific_workflow
29+
- added|modified: '.github/file-filters.yml'
30+
- added|modified: 'package.json'
31+
- added|modified: 'yarn.lock'
32+
- added|deleted|modified: '.prettierignore'
33+
- added|deleted|modified: '.eslintignore'
34+
- added|deleted|modified: '.eslint*'
35+
36+
# Provides list of changed files to test (jest)
37+
# getsentry/sentry does not use this directly, instead we shard tests inside jest.config.js
38+
testable_modified: &testable_modified
39+
- added|modified: 'package.json'
40+
- added|modified: 'static/**/*.[tj]{s,sx}'
41+
- *sentry_specific_test_files
42+
43+
# Trigger for when we must run full tests (jest)
44+
testable_rules_changed: &testable_rules_changed
45+
- *sentry_specific_workflow
46+
- added|modified: '.github/file-filters.yml'
47+
- added|modified: 'jest.config.ts'
48+
49+
# Trigger whether to run tsc or not
50+
# There's no "rules_changed" for this, because we run it for all files always
51+
# getsentry/sentry does not use this directly, instead frontend_all is a superset to trigger tsc
52+
typecheckable_rules_changed: &typecheckable_rules_changed
53+
- *sentry_specific_workflow
54+
- added|modified: '.github/file-filters.yml'
55+
- added|deleted|modified: '**/tsconfig*.json'
56+
- added|deleted|modified: 'static/**/*.[tj]{s,sx}'
57+
58+
# Trigger to apply the 'Scope: Frontend' label to PRs
3359
frontend_all: &frontend_all
34-
- *frontend_src
35-
- '**/*.[tj]{s,sx}'
36-
37-
frontend_modified_lintable:
38-
- added|modified: *frontend_lintable
39-
40-
frontend_components_modified_lintable:
41-
- added|modified: *frontend_components_lintable
60+
- *lintable_css_in_js_rules_changed
61+
- *lintable_rules_changed
62+
- *testable_rules_changed
63+
- *typecheckable_rules_changed
64+
- added|modified: '{.volta,vercel}.json'
65+
- *sentry_specific_workflow
66+
- *sentry_specific_test_files
4267

4368
# Also used in `getsentry-dispatch.yml` to dispatch backend tests on getsentry
4469
backend_dependencies: &backend_dependencies

.github/workflows/frontend.yml

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@ jobs:
2424
timeout-minutes: 3
2525
# Map a step output to a job output
2626
outputs:
27-
eslint_config: ${{ steps.changes.outputs.eslint_config }}
28-
frontend: ${{ steps.changes.outputs.frontend_all }}
29-
frontend_components_modified_lintable: ${{ steps.changes.outputs.frontend_components_modified_lintable }}
30-
frontend_components_modified_lintable_files: ${{ steps.changes.outputs.frontend_components_modified_lintable_files }}
31-
frontend_modified_lintable_files: ${{ steps.changes.outputs.frontend_modified_lintable_files }}
32-
yarn_lockfile: ${{ steps.changes.outputs.yarn_lockfile }}
27+
lintable_css_in_js_modified: ${{ steps.changes.outputs.lintable_css_in_js_modified }}
28+
lintable_css_in_js_modified_files: ${{ steps.changes.outputs.lintable_css_in_js_modified_files }}
29+
lintable_css_in_js_rules_changed: ${{ steps.changes.output.lintable_css_in_js_rules_changed }}
30+
lintable_modified: ${{ steps.changes.outputs.lintable_modified }}
31+
lintable_modified_files: ${{ steps.changes.outputs.lintable_modified_files }}
32+
lintable_rules_changed: ${{ steps.changes.outputs.lintable_rules_changed }}
33+
testable_modified: ${{ steps.changes.outputs.testable_modified }}
34+
testable_modified_files: ${{ steps.changes.outputs.testable_modified_files }}
35+
testable_rules_changed: ${{ steps.changes.outputs.testable_rules_changed }}
36+
frontend_all: ${{ steps.changes.outputs.frontend_all }}
3337
steps:
3438
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
3539

@@ -42,7 +46,7 @@ jobs:
4246
list-files: shell
4347

4448
typescript-and-lint:
45-
if: needs.files-changed.outputs.frontend == 'true'
49+
if: needs.files-changed.outputs.frontend_all == 'true'
4650
needs: files-changed
4751
name: typescript and lint
4852
runs-on: ubuntu-22.04
@@ -70,49 +74,52 @@ jobs:
7074
echo "::add-matcher::.github/tsc.json"
7175
echo "::add-matcher::.github/eslint-stylish.json"
7276
73-
- name: eslint logic
74-
id: eslint
75-
if: (github.ref == 'refs/heads/master' || needs.files-changed.outputs.eslint_config == 'true' || needs.files-changed.outputs.yarn_lockfile == 'true')
76-
run: echo "all-files=true" >> "$GITHUB_OUTPUT"
77-
78-
# Lint entire frontend if:
79-
# - this is on main branch
80-
# - eslint configuration in repo has changed
81-
# - yarn lockfile has changed (i.e. we bump our eslint config)
82-
- name: eslint
83-
if: steps.eslint.outputs.all-files == 'true'
84-
env:
85-
# Run relax config on main branch (and stricter config for changed files)
86-
SENTRY_ESLINT_RELAXED: 1
87-
run: yarn lint
77+
# When we're on master we can run all tasks across all files
78+
# or if lint rules have changed, run the related task across all files
79+
- name: biome (all files)
80+
if: github.ref == 'refs/heads/master'
81+
run: yarn lint:biome
82+
83+
- name: prettier (all files)
84+
if: github.ref == 'refs/heads/master' || needs.files-changed.outputs.lintable_rules_changed == 'true'
85+
run: yarn lint:prettier
86+
87+
- name: eslint (all files)
88+
if: github.ref == 'refs/heads/master' || needs.files-changed.outputs.lintable_rules_changed == 'true'
89+
run: yarn lint:js
90+
91+
- name: stylelint (all files)
92+
if: github.ref == 'refs/heads/master' || needs.files-changed.outputs.lintable_css_in_js_rules_changed == 'true'
93+
run: yarn lint:css
94+
95+
# When on a PR branch, we only need to look at the changed files
96+
- name: biome (fix)
97+
if: github.ref != 'refs/heads/master'
98+
run: yarn fix:biome
99+
100+
- name: prettier (changed files only)
101+
if: github.ref != 'refs/heads/master' && needs.files-changed.outputs.lintable_rules_changed != 'true' && needs.files-changed.outputs.lintable_modified == 'true'
102+
run: yarn prettier --write ${{ needs.files-changed.outputs.lintable_modified_files }}
88103

89-
# Otherwise... only lint modified files
90-
# Note `eslint --fix` will not fail when it auto fixes files
91104
- name: eslint (changed files only)
92-
if: steps.eslint.outputs.all-files != 'true'
93-
run: |
94-
yarn eslint --fix ${{ needs.files-changed.outputs.frontend_modified_lintable_files }}
105+
if: github.ref != 'refs/heads/master' && needs.files-changed.outputs.lintable_rules_changed != 'true' && needs.files-changed.outputs.lintable_modified == 'true'
106+
run: yarn eslint --fix ${{ needs.files-changed.outputs.lintable_modified_files }}
95107

96108
- name: stylelint (changed files only)
97-
if: github.ref != 'refs/heads/master' && needs.files-changed.outputs.frontend_components_modified_lintable == 'true'
98-
run: |
99-
yarn stylelint ${{ needs.files-changed.outputs.frontend_components_modified_lintable_files }}
100-
101-
- name: biome
102-
if: github.ref != 'refs/heads/master' && needs.files-changed.outputs.frontend_components_modified_lintable == 'true'
103-
run: yarn biome check . --apply
109+
if: github.ref != 'refs/heads/master' && needs.files-changed.outputs.lintable_css_in_js_rules_changed != 'true' && needs.files-changed.outputs.lintable_css_in_js_modified == 'true'
110+
run: yarn stylelint ${{ needs.files-changed.outputs.lintable_css_in_js_modified_files }}
104111

105112
# Check (and error) for dirty working tree for forks
106113
# Reason being we need a different token to auto commit changes and
107114
# forks do not have access to said token
108115
- name: Check for dirty git working tree (forks)
109-
if: steps.token.outcome != 'success' && github.ref != 'refs/heads/master'
116+
if: github.ref != 'refs/heads/master' && steps.token.outcome != 'success'
110117
run: |
111118
git diff --quiet || (echo '::error ::lint produced file changes, run linter locally and try again' && exit 1)
112119
113120
# If working tree is dirty, commit and update if we have a token
114121
- name: Commit any eslint fixed files
115-
if: steps.token.outcome == 'success' && github.ref != 'refs/heads/master'
122+
if: github.ref != 'refs/heads/master' && steps.token.outcome == 'success'
116123
uses: getsentry/action-github-commit@31f6706ca1a7b9ad6d22c1b07bf3a92eabb05632 # v2.0.0
117124
with:
118125
github-token: ${{ steps.token.outputs.token }}
@@ -124,7 +131,7 @@ jobs:
124131
run: yarn tsc -p config/tsconfig.ci.json
125132

126133
frontend-jest-tests:
127-
if: needs.files-changed.outputs.frontend == 'true'
134+
if: needs.files-changed.outputs.testable_rules_changed == 'true' || needs.files-changed.outputs.testable_modified == 'true'
128135
needs: files-changed
129136
name: Jest
130137
# If you change the runs-on image, you must also change the runner in jest-balance.yml
@@ -137,6 +144,7 @@ jobs:
137144
fail-fast: false
138145
matrix:
139146
# XXX: When updating this, make sure you also update CI_NODE_TOTAL.
147+
140148
instance: [0, 1, 2, 3]
141149

142150
steps:
@@ -186,7 +194,7 @@ jobs:
186194
# it reduces large coverage fluctuations.
187195
- name: Handle artifacts
188196
uses: ./.github/actions/artifacts
189-
if: ${{ always() && needs.files-changed.outputs.frontend_all == 'true' }}
197+
if: always()
190198
with:
191199
files: .artifacts/coverage/*
192200
type: frontend

.github/workflows/label-pullrequest.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ jobs:
2525

2626
- name: Add frontend label
2727
uses: getsentry/action-add-labels@54d0cba498c1eaf8bd34985d715504d1b6e2935f
28-
if: steps.changes.outputs.frontend_src == 'true'
28+
if: steps.changes.outputs.frontend_all == 'true'
2929
with:
3030
labels: 'Scope: Frontend'
3131

@@ -46,7 +46,7 @@ jobs:
4646
- name: Add frontend/backend warning comment
4747
uses: peter-evans/create-or-update-comment@b95e16d2859ad843a14218d1028da5b2c4cbc4b4
4848
if: >
49-
steps.changes.outputs.frontend_src == 'true' &&
49+
steps.changes.outputs.frontend_all == 'true' &&
5050
steps.changes.outputs.backend_src == 'true' &&
5151
steps.fc.outputs.comment-id == 0
5252
with:

package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,12 @@
223223
"test-precommit": "yarn test-wrap --bail --findRelatedTests -u",
224224
"test-staged": "yarn test-wrap --findRelatedTests $(git diff --name-only --cached)",
225225
"lint": "yarn lint:biome && yarn lint:prettier && yarn lint:js && yarn lint:css",
226-
"lint:js": "eslint static/app tests/js --ext .js,.jsx,.ts,.tsx",
227-
"lint:css": "stylelint 'static/app/**/*.[jt]sx'",
226+
"lint:js": "eslint . --ext .js,.jsx,.ts,.tsx",
227+
"lint:css": "stylelint '**/*.[jt]sx'",
228228
"lint:biome": "biome check .",
229229
"lint:prettier": "prettier \"**/*.md\" \"**/*.yaml\" \"**/*.[jt]s(x)?\" --check",
230230
"fix": "yarn fix:biome && yarn fix:prettier && yarn fix:eslint",
231-
"fix:eslint": "eslint static/app tests/js --ext .js,.jsx,.ts,.tsx --fix",
231+
"fix:eslint": "eslint . --ext .js,.jsx,.ts,.tsx --fix",
232232
"fix:biome": "biome check . --apply",
233233
"fix:prettier": "prettier \"**/*.md\" \"**/*.yaml\" \"**/*.[jt]s(x)?\" --write",
234234
"dev": "(yarn check --verify-tree || yarn install --check-files) && sentry devserver",

scripts/extract-android-device-names.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const fs = require('node:fs');
33

44
const transformResults = res => {
55
const deviceMapping = {};
6-
res.map(({name, model}) => {
6+
res.forEach(({name, model}) => {
77
if (name && model) {
88
deviceMapping[model] = name;
99
}

scripts/test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ process.on('unhandledRejection', err => {
1313
throw err;
1414
});
1515

16-
// eslint-disable-next-line jest/no-jest-import
1716
const jest = require('jest');
1817

1918
let argv = process.argv.slice(2);

0 commit comments

Comments
 (0)