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

extension whitelist feature #143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mightyiam
Copy link
Contributor

No description provided.

@mightyiam
Copy link
Contributor Author

Corresponding with standard/standard#703.

@mightyiam
Copy link
Contributor Author

I can see that I've implemented this feature when whitelist is provided in new Linter(opts), and not in Linter.prototype.lintText nor Linter.prototype.lintFiles.

Question: why does the API allow for providing opts to lintText and lintFiles versus only on instantiation? This seems unusual to me and it does not mimic ESLint's executeOnText nor executeOnFiles. So I don't get it.

Question: both lintText and lintFiles use parseOpts to parse the opts. Why does the Linter constructor not do this, please? If it did, I could have added the feature strictly in parseOpts. That seems the reasonable thing to do, doesn't it? Should I make a prior PR in which the constructor uses parseOpts?

Question: Is it time to move forward and drop node v4 or should I add strict mode or should I target non-strict node v4?

🐱

@toddbluhm
Copy link
Contributor

toddbluhm commented Apr 30, 2020

Question: why does the API allow for providing opts to lintText and lintFiles versus only on instantiation? This seems unusual to me and it does not mimic ESLint's executeOnText nor executeOnFiles. So I don't get it.

I really don't know why this is the case. Perhaps to allow functional usage of just using lintText or lintFiles without having to call parseOpts or a constructor?

Question: both lintText and lintFiles use parseOpts to parse the opts. Why does the Linter constructor not do this, please? If it did, I could have added the feature strictly in parseOpts. That seems the reasonable thing to do, doesn't it? Should I make a prior PR in which the constructor uses parseOpts?

I agree in that I think we should unify the options parsing. We should just use parseOpts to parse both the constructor opts and the lintText/lintFiles opts. That should allow it to maintain backwards compatibility. This would mean all options parsing/setting would only need to be implemented one time in parseOpts. Maybe at a later date, we can look into modernizing the exported API to allow opts initialization only in the constructor.

Question: Is it time to move forward and drop node v4 or should I add strict mode or should I target non-strict node v4?

We should definitely drop node v4 support and migrate up to a minimum node v8 (Edit: fixed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants