New: implement eslint/rfcs#63 #13713

billiegoose · 2020-09-23T21:40:53Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[x] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

This is heavily based on #11487 but as per the RFC the names of the options have been changed.

I also rebased it and tried to get all the tests passing. There's still one failing locally. Not sure what that's about yet.

Is there anything you'd like reviewers to focus on?

My original PR was a one-liner. And now my PR is largely someone elses code 😬 so aaaaany help would be appreciated. I don't care about credit, I just want this feature shipped so my team stops burning through CircleCI minutes waiting to lint files that don't need to be linted.

background

Currently CI users resort to hacks such as manually modifying the mtime of all their files:
https://github.com/salto-io/salto/pull/395/files#diff-5d8151d7fb61e762279563e3d38abd41R1-R17

Previous attempts to address this:

jsf-clabot · 2020-09-23T21:40:55Z

All committers have signed the CLA.

nzakas · 2020-09-24T00:47:22Z

Thanks for attempting this. I’m not comfortable with reading an environment variable for this purpose — that’s just one assumption too many when determining how ESLint should behave.

We did recently merge this RFC that covers this use case: eslint/rfcs#63

billiegoose · 2020-09-24T03:03:15Z

Oh that's fantastic! That's great. Is there already an implementation of RFC 63? If not, I can pivot to implementing it.

billiegoose · 2020-09-24T17:46:01Z

@c-home anything I can do to help?

nzakas · 2020-09-24T20:06:04Z

I don't believe implementation has started yet, so you can feel free!

eslint-deprecated · 2020-09-25T01:53:47Z

Hi @wmhilton!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag must be one of the following:

The Tag is one of the following:
- Fix - for a bug fix.
- Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
- New - implements a new feature.
- Breaking - for a backwards-incompatible enhancement or feature.
- Docs - changes to documentation only.
- Build - changes to build process only.
- Upgrade - for a dependency upgrade.
- Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).
You can use the labels of the issue you are working on to determine the best tag.

Read more about contributing to ESLint here

eslint-deprecated · 2020-09-25T01:54:07Z

Hi @wmhilton!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag must be one of the following:

The Tag is one of the following:
- Fix - for a bug fix.
- Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
- New - implements a new feature.
- Breaking - for a backwards-incompatible enhancement or feature.
- Docs - changes to documentation only.
- Build - changes to build process only.
- Upgrade - for a dependency upgrade.
- Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).
You can use the labels of the issue you are working on to determine the best tag.

Read more about contributing to ESLint here

eslint-deprecated · 2020-09-25T02:00:08Z

Hi @wmhilton!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag must be one of the following:

The Tag is one of the following:
- Fix - for a bug fix.
- Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
- New - implements a new feature.
- Breaking - for a backwards-incompatible enhancement or feature.
- Docs - changes to documentation only.
- Build - changes to build process only.
- Upgrade - for a dependency upgrade.
- Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).
You can use the labels of the issue you are working on to determine the best tag.

Read more about contributing to ESLint here

c-home · 2020-09-25T02:35:03Z

@c-home anything I can do to help?

Hi, I started implementing the RFC but haven't gotten a chance to add testing yet (which is really the most of it).
If you would like to implement it that is great :)

nzakas

Sorry for the delay in reviewing, this looks really good. Just a few small things to clean up and I'd like to get a few other sets of eyes on this, too.

docs/user-guide/command-line-interface.md

lib/eslint/eslint.js

lib/options.js

nzakas · 2020-10-07T17:53:27Z

@c-home if you have the time to take a look at this implementation, we'd appreciate your feedback.

c-home · 2020-10-08T06:28:16Z

@c-home if you have the time to take a look at this implementation, we'd appreciate your feedback.

Had a quick look and it looks great. Will have a closer look soon. Thank you for implementing this wmhilton.

c-home · 2020-10-08T23:36:22Z

lib/cli-engine/lint-result-cache.js

+        const changed =
+            fileDescriptor.changed ||
+            fileDescriptor.meta.hashOfConfig !== hashOfConfig;


Will changed be true if the cacheStrategy changes? Related to this issue raised in the rfc.

c-home · 2020-10-11T08:19:39Z

For users of eslint cache, this is something to consider - eslint/rfcs#68.

eslint-deprecated · 2020-10-29T14:42:21Z

Hi @wmhilton!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The issue reference must be formatted as follows:

If the pull request addresses an issue, then the issue number should be mentioned at the end. If the commit doesn't completely fix the issue, then use (refs #1234) instead of (fixes #1234).

Here are some good commit message summary examples:
```
Build: Update Travis to only test Node 0.10 (refs #734)
Fix: Semi rule incorrectly flagging extra semicolon (fixes #840)
Upgrade: Esprima to 1.2, switch to using comment attachment (fixes #730)
```

Read more about contributing to ESLint here

billiegoose · 2020-10-29T14:58:54Z

Ugh. As pointed out in this comment some very notable rulesets such as @typescript-eslint are incompatible with --cache by design. That really puts a damper on my enthusiasm for this, since anything less than 100% reproducible isn't good for our CI use case at Stoplight. :-(

nzakas · 2020-10-29T20:06:57Z

It seems like you are pretty close to finishing this, and this is something we plan on doing regardless. Do you want to finish it up so we can merge it?

We do still need to solve for the case where plugins are doing unexpected things, but there are plenty of folks who would benefit from this PR in the meantime.

royriojas · 2020-11-08T10:47:09Z

hi @nzakas @wmhilton I've published a new version of file-entry-cache. Version 6.0.0. This version has no new functionality but updates the dependencies to latest to remove security warnings.

It is a major upgrade to avoid any potential issues since I've aligned the supported node versions with the ones eslint is currently using.

nzakas · 2020-11-11T19:28:32Z

@royriojas thanks for the heads up. Can you open an issue so that info doesn’t get lost here?

vaniyokk · 2021-02-07T11:34:07Z

Any update on this? Really annoying to wait minutes on CI....(

nzakas · 2021-02-09T01:46:46Z

@vaniyokk it appears this has been abandoned. Do you want to take it up?

kirill-konshin · 2021-02-09T03:40:24Z

@vaniyokk it appears this has been abandoned. Do you want to take it up?

This is quite sad. In our case on CI with repo having around 10K modules ESLint takes waaaay too much time, since the cache is not used at all between pushes (due to timestamps inconsistency, Git does not maintain them, and GitLab does not have workspaces so far).

royriojas · 2021-02-09T07:08:02Z

The merge request seems to be almost ready, isn't it? @nzakas. I think it is only a matter of fix the tests

snitin315 · 2021-02-09T08:10:57Z

Can I work on this and finish the PR ???

nzakas · 2021-02-09T18:48:53Z

@royriojas yes, it looks basically done at this point.

@snitin315 yes, you can definitely take it over. You should be able to pull the branch from where this PR was created and then create a new PR with that code. When you do, just comment here with the new PR number so we can close this and move our focus over to the new PR. Thanks!

mdjermanovic · 2021-02-18T11:46:17Z

Closing as there's a new PR #14119

eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Sep 23, 2020

billiegoose force-pushed the feat/ci-caching-no-mtime branch from 09174b0 to 8c0ed1a Compare September 25, 2020 01:53

billiegoose changed the title ~~Fix: --cache ignores file mtime in CI environments~~ Feat: implementation of eslint/rfcs#63 Sep 25, 2020

billiegoose force-pushed the feat/ci-caching-no-mtime branch from 8c0ed1a to bafe281 Compare September 25, 2020 02:01

nzakas changed the title ~~Feat: implementation of eslint/rfcs#63~~ New: implement eslint/rfcs#63 Oct 7, 2020

nzakas requested changes Oct 7, 2020

View reviewed changes

docs/user-guide/command-line-interface.md Outdated Show resolved Hide resolved

lib/eslint/eslint.js Show resolved Hide resolved

lib/options.js Outdated Show resolved Hide resolved

mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint and removed triage An ESLint team member will look at this issue soon labels Oct 8, 2020

c-home reviewed Oct 8, 2020

View reviewed changes

mdjermanovic mentioned this pull request Oct 9, 2020

ESLint ignores its own cache #13517

Closed

Update: Implement eslint/rfcs#63

760eb5c

billiegoose force-pushed the feat/ci-caching-no-mtime branch from ef2c62c to 760eb5c Compare October 29, 2020 14:54

royriojas mentioned this pull request Nov 8, 2020

Allow setting useChecksum via env var jaredwray/file-entry-cache#14

Closed

chambo-e mentioned this pull request Feb 16, 2021

New: Implement cacheStrategy (refs eslint/rfcs#63) #14119

Merged

1 task

mdjermanovic closed this Feb 18, 2021

eslint-github-bot bot locked and limited conversation to collaborators Aug 18, 2021

eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New: implement eslint/rfcs#63 #13713

New: implement eslint/rfcs#63 #13713

billiegoose commented Sep 23, 2020 •

edited

jsf-clabot commented Sep 23, 2020 •

edited

nzakas commented Sep 24, 2020

billiegoose commented Sep 24, 2020

billiegoose commented Sep 24, 2020

nzakas commented Sep 24, 2020

eslint-deprecated bot commented Sep 25, 2020

eslint-deprecated bot commented Sep 25, 2020

eslint-deprecated bot commented Sep 25, 2020

c-home commented Sep 25, 2020 •

edited

nzakas left a comment

nzakas commented Oct 7, 2020

c-home commented Oct 8, 2020

c-home Oct 8, 2020

c-home commented Oct 11, 2020

eslint-deprecated bot commented Oct 29, 2020

billiegoose commented Oct 29, 2020

nzakas commented Oct 29, 2020

royriojas commented Nov 8, 2020

nzakas commented Nov 11, 2020

vaniyokk commented Feb 7, 2021 •

edited

nzakas commented Feb 9, 2021

kirill-konshin commented Feb 9, 2021

royriojas commented Feb 9, 2021

snitin315 commented Feb 9, 2021

nzakas commented Feb 9, 2021

mdjermanovic commented Feb 18, 2021

New: implement eslint/rfcs#63 #13713

New: implement eslint/rfcs#63 #13713

Conversation

billiegoose commented Sep 23, 2020 • edited

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

background

jsf-clabot commented Sep 23, 2020 • edited

nzakas commented Sep 24, 2020

billiegoose commented Sep 24, 2020

billiegoose commented Sep 24, 2020

nzakas commented Sep 24, 2020

eslint-deprecated bot commented Sep 25, 2020

eslint-deprecated bot commented Sep 25, 2020

eslint-deprecated bot commented Sep 25, 2020

c-home commented Sep 25, 2020 • edited

nzakas left a comment

Choose a reason for hiding this comment

nzakas commented Oct 7, 2020

c-home commented Oct 8, 2020

c-home Oct 8, 2020

Choose a reason for hiding this comment

c-home commented Oct 11, 2020

eslint-deprecated bot commented Oct 29, 2020

billiegoose commented Oct 29, 2020

nzakas commented Oct 29, 2020

royriojas commented Nov 8, 2020

nzakas commented Nov 11, 2020

vaniyokk commented Feb 7, 2021 • edited

nzakas commented Feb 9, 2021

kirill-konshin commented Feb 9, 2021

royriojas commented Feb 9, 2021

snitin315 commented Feb 9, 2021

nzakas commented Feb 9, 2021

mdjermanovic commented Feb 18, 2021

billiegoose commented Sep 23, 2020 •

edited

jsf-clabot commented Sep 23, 2020 •

edited

c-home commented Sep 25, 2020 •

edited

vaniyokk commented Feb 7, 2021 •

edited