feat(linter): Add support for ignores property within config file #7092
Conversation
Your org has enabled the Graphite merge queue for merging into mainAdd the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
github-actions
bot
added
A-linter
| let paths = Walk::new( | ||
| &paths, | ||
| &ignore_options, | ||
| &oxlintrc.ignores.iter().map(|value| cwd.join(value)).collect::<Vec<_>>(), |
There was a problem hiding this comment.
This joins with the current working directory, but maybe it should be based on the config file path?
There was a problem hiding this comment.
Using the config file path is also problematic because you might store all your config in a configs folder, so then all your globs would need to traverse up to get out of it.
Maybe the current working directory is the easiest to explain right now. When thinking about nested config files I feel like it becomes less clear though.
There was a problem hiding this comment.
Usually use case:
- simple project: eslint config in root folder
- Monorepo setup: each folder (library or project) can contain eslint config extended from base root config file and then ovverride or add own rules and etc
Usually ignores used mostly from root config
| .add_line(None, path) | ||
| .expect("Invalid glob in config \"ignores\" property"); | ||
| } | ||
| self.gitignore_glob.lock().await.push(gitignore_builder.build().unwrap()); |
There was a problem hiding this comment.
This modifies self.gitignore_glob after it has been created in init_ignore_glob. This might be confusing and things could probably be refactored so that it isn't modified outside of init_ignore_glob.
CodSpeed Performance ReportMerging #7092 will not alter performanceComparing Summary
|
nrayburn-tech
force-pushed
the
support-ignores-config
branch
from
c4e2d96
to
a70b833
Compare
|
@Boshen Could you please a look at this PR? |
This could probably use some tests, but I'm not really sure what exactly should be tested.
Will leave a review with a few comments on things that might need a different approach.
Closes #7032.