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

New: Variable Definition Information of Config Files #17

Merged

Conversation

@mysticatea
Copy link
Member

@mysticatea mysticatea commented Mar 4, 2019

Link to Rendered RFC

Summary

This proposal adds variable definition information of config files to Variable objects.

Related Issues

Copy link
Member

@platinumazure platinumazure left a comment

This looks good to me in general.

Do any of the existing Variable properties need to be deprecated? (Maybe I missed that.)

Copy link
Member

@not-an-aardvark not-an-aardvark left a comment

This generally looks good to me, I have a few minor comments.

not-an-aardvark and others added 2 commits Mar 11, 2019
…EADME.md

Co-Authored-By: mysticatea <star.ctor@gmail.com>
- remove `variable.eslintExplicitGlobalComment` and analysis for bc.
@mysticatea
Copy link
Member Author

@mysticatea mysticatea commented Mar 11, 2019

Thank you for the review.

I updated this RFC:

  • it removes existing variable.eslintExplicitGlobalComment property in favor of new variable.eslintExplicitGlobalComments property.
  • it describes documentation details.
  • it describes backward compatibility analysis about that removal.

Copy link
Member

@platinumazure platinumazure left a comment

This looks good to me. Thanks!

@platinumazure
Copy link
Member

@platinumazure platinumazure commented Mar 11, 2019

As a note for implementation, I think we could add the new properties in a semver-minor release and remove the deprecated property in a major release, if needed. (Only noting in case this doesn't make 6.0.0.) Does anyone disagree?

@nzakas
Copy link
Member

@nzakas nzakas commented Mar 11, 2019

@platinumazure I agree. I don't think there's a reason to do this as a breaking change.

nzakas
nzakas approved these changes Mar 11, 2019
Copy link
Member

@nzakas nzakas left a comment

Make sense to me. Nice work! 👍

@not-an-aardvark
Copy link
Member

@not-an-aardvark not-an-aardvark commented Mar 11, 2019

To be clear, while the core changes in this RFC could be done without a breaking change, modifying the defaults of no-redeclare would require a breaking change, and I think that was the main motivation for this proposal. So we would need to do this before v6.0.0 if we want the no-redeclare change to be in v6.

@nzakas
Copy link
Member

@nzakas nzakas commented Mar 13, 2019

@not-an-aardvark
Copy link
Member

@not-an-aardvark not-an-aardvark commented Mar 13, 2019

We already accepted the proposal (pending an RFC) as a breaking change for v6.0.0 in the previous TSC meeting.

@nzakas
Copy link
Member

@nzakas nzakas commented Mar 13, 2019

mysticatea added a commit to eslint/eslint that referenced this issue Mar 15, 2019
- Implement eslint/rfcs#17
- Fixes #11370
- Remvoes the access to parserOptions from no-redeclare rule
- Adds several tests for lexical bindings to no-redeclare rule
@mysticatea
Copy link
Member Author

@mysticatea mysticatea commented Mar 28, 2019

Hi. Thank you for the feedback.

I labeled this RFC as "Final Commenting" because over 21 days satisfied.
For a reference, the implementation exists on eslint/eslint#11509.

cc: @eslint/eslint-tsc

Copy link
Member

@btmills btmills left a comment

Makes sense!

@mysticatea
Copy link
Member Author

@mysticatea mysticatea commented Apr 5, 2019

OK, I will merge this RFC because 7 days after final commenting satisfied and looks no objections.

Thank you very much!

@mysticatea mysticatea merged commit 26af0a9 into master Apr 5, 2019
3 checks passed
@mysticatea mysticatea deleted the 2019-variable-definition-information-of-config-files branch Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants