-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: Add includeIgnoreFile() method #47
Conversation
packages/compat/README.md
Outdated
import { includeIgnoreFile } from "@eslint/compat"; | ||
|
||
export default [ | ||
includeIgnoreFile(".gitignore"), |
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.
I think it would be better to recommend passing an absolute path here. Relative paths are resolved relative to process.cwd()
, so whether this will work or not depends on where eslint is run from.
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.
Ah right. 🤦 Second time this week I forgot that fact.
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.
Just one typo, and there's also a merge conflict.
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
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.
LGTM, thanks!
Prerequisites checklist
What is the purpose of this pull request?
Add a
includeIgnoreFile()
function.What changes did you make? (Give an overview)
includedIgnoreFile()
function that is exported from@eslint/compat
convertIgnorePatternToMinimatch()
function that is exported from@eslint/compat
(purposely undocumented because I'm not sure how end users would use this)@eslint/compat
@eslint/migrate-config
to use theconvertIgnorePatternToMinimatch()
function from@eslint/compat
instead of duplicating that functionality@eslint/migrate-config
comes after@eslint/compat
due to the new dependency on itNote: The
includeIgnoreFile()
function differs from the original issue in that there is no second argument. I'm not sure it's needed and it could complicate the process if the ignore file is in an ancestor directory. For this first release, I'd like to see what kind of feedback we get as-is.Related Issues
fixes #44
Is there anything you'd like reviewers to focus on?