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

Add ESLint and Stylelint caches to GitHub actions workflow #2860

Merged
merged 2 commits into from Sep 30, 2022

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Sep 20, 2022

During workflow runs we only need to lint "changed" files

Both ESLint and Stylelint support this 👍

This removes a 2-3 second delay during both gulp dev file changes (Sass and JS files) and git hook "lint" runs

Before

GitHub Actions workflow

Lint:              13s
Test projects:     1m 25s
Screenshots:       1m 24s
Total:             3m 2s
npm run lint:js    4.87s user 0.44s system 141% cpu 3.752 total
npm run lint:scss  4.91s user 0.65s system 144% cpu 3.845 total

After

GitHub Actions workflow (~49s faster with Jest, ESLint and Stylelint caching)

Lint:              4s
Test projects:     1m 1s
Screenshots:       1m 8s
Total:             2m 13s
npm run lint:js    1.16s user 0.36s system 105% cpu 1.438 total
npm run lint:scss  2.41s user 0.56s system 137% cpu 2.173 total

@colinrotherham colinrotherham requested a review from a team as a code owner September 20, 2022 08:03
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2860 September 20, 2022 08:03 Inactive
@colinrotherham colinrotherham added this to Needs review 🔍 in Design System Sprint Board via automation Sep 20, 2022
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2860 September 20, 2022 09:53 Inactive
package.json Outdated
@@ -23,7 +23,7 @@
"test:build:dist": "npx jest --selectProjects \"Gulp tasks\" --testMatch \"**/*build-dist*\"",
"lint": "npm run lint:js && npm run lint:scss",
"lint:js": "eslint --cache --cache-location .cache/eslint --cache-strategy content --color --ignore-path .gitignore \"**/*.{cjs,js,mjs}\"",
"lint:scss": "stylelint \"app/**/*.scss\" \"src/**/*.scss\""
"lint:scss": "stylelint --cache --cache-location .cache/stylelint --color --ignore-path .gitignore \"app/**/*.scss\" \"src/**/*.scss\""
Copy link
Member

Choose a reason for hiding this comment

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

If we changed the stylelint config, will we need to clear the cache to force it to re-lint all files against the new config? I can't see anything in the stylelint docs to suggest this is handled for us.

(Same applies for eslint)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ESLint automatically marks the cache as stale when the config changes

Let me check what StyleLint does (good shout)

Update: StyleLint doesn't yet but it's been picked up in stylelint/stylelint#2908 (comment)

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2860 September 20, 2022 18:55 Inactive
@colinrotherham colinrotherham marked this pull request as draft September 21, 2022 10:57
@colinrotherham
Copy link
Contributor Author

colinrotherham commented Sep 21, 2022

Converting back to draft whilst we wait for stylelint/stylelint#6356 🎉

@36degrees 36degrees moved this from Needs review 🔍 to Blocked ⛔ in Design System Sprint Board Sep 28, 2022
@jeddy3
Copy link

jeddy3 commented Sep 28, 2022

Converting back to draft whilst we wait for stylelint/stylelint#6356 🎉

We've just released stylelint@14.13.0 with this fix in. It also contains a new --cache-strategy content flag that you may want to use for parity with how you use ESLint.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2860 September 28, 2022 11:28 Inactive
@colinrotherham
Copy link
Contributor Author

We've just released stylelint@14.13.0 with this fix in. It also contains a new --cache-strategy content flag that you may want to use for parity with how you use ESLint.

Thanks @jeddy3 (and @kimulaco @kaorun343 @ybiquitous)

Massively appreciate the new addition (and contribution) 🙌

@colinrotherham colinrotherham changed the base branch from main to reduce-percy-usage September 28, 2022 16:59
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2860 September 28, 2022 17:00 Inactive
@colinrotherham colinrotherham marked this pull request as ready for review September 28, 2022 18:58
@colinrotherham
Copy link
Contributor Author

@36degrees This one's ready to review again 😊

Not a huge speed bump for the GitHub workflow, but noticeable when running gulp dev (touching lots of Sass files)

Base automatically changed from reduce-percy-usage to main September 30, 2022 10:22
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2860 September 30, 2022 10:29 Inactive
@colinrotherham colinrotherham merged commit 0b07a7f into main Sep 30, 2022
Design System Sprint Board automation moved this from Blocked ⛔ to Done 🏁 Sep 30, 2022
@colinrotherham colinrotherham deleted the stylelint-cache branch September 30, 2022 15:18
@colinrotherham colinrotherham added the github_actions Pull requests that update GitHub Actions code label Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update GitHub Actions code tooling
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants