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

Jump to bottom

Breaking: description in directive comments (refs eslint/rfcs#33) #12699

Merged
merged 10 commits into from Jan 17, 2020
Merged

Breaking: description in directive comments (refs eslint/rfcs#33) #12699

merged 10 commits into from Jan 17, 2020

Conversation

mysticatea
Copy link
Member

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

[X] Add something to the core: add the support of comments in directive comments.

What changes did you make? (Give an overview)

This PR implements RFC33.

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

Are there more proper places for documentation?

@mysticatea mysticatea added enhancement This change enhances an existing feature of ESLint core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion breaking This change is backwards-incompatible do not merge This pull request should not be merged yet labels Dec 22, 2019
@mysticatea mysticatea added this to Implemented, pending review in v7.0.0 Dec 22, 2019
@mysticatea mysticatea added this to Implemented, pending review in RFCs Dec 22, 2019
@kaicataldo kaicataldo removed the do not merge This pull request should not be merged yet label Dec 23, 2019
kaicataldo
kaicataldo reviewed Dec 24, 2019
View reviewed changes
Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few suggestions around the documentation, but I think this is looking great! Thanks for working on this.

docs/user-guide/configuring.md Outdated Show resolved Hide resolved
docs/user-guide/configuring.md Outdated Show resolved Hide resolved
docs/user-guide/configuring.md Outdated Show resolved Hide resolved
docs/user-guide/configuring.md Outdated Show resolved Hide resolved
docs/user-guide/configuring.md Outdated Show resolved Hide resolved
docs/user-guide/configuring.md Outdated Show resolved Hide resolved
tests/lib/linter/linter.js Outdated Show resolved Hide resolved
mysticatea and others added 6 commits December 24, 2019 15:17
@mysticatea @kaicataldo
Update docs/user-guide/configuring.md
ed9ca64 
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
@mysticatea @kaicataldo
Update docs/user-guide/configuring.md
4246f02 
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
@mysticatea @kaicataldo
Update docs/user-guide/configuring.md
eb32ec7 
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
@mysticatea @kaicataldo
Update docs/user-guide/configuring.md
7808a32 
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
@mysticatea @kaicataldo
Update docs/user-guide/configuring.md
057befe 
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
@mysticatea @kaicataldo
Update tests/lib/linter/linter.js
b6d61d6 
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
kaicataldo
kaicataldo reviewed Jan 6, 2020
View reviewed changes
Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed the typos in my original suggestion. Other than these comments, LGTM!

docs/user-guide/configuring.md Outdated Show resolved Hide resolved
docs/user-guide/configuring.md Outdated Show resolved Hide resolved
@mysticatea
Copy link
Member Author

Thank you for your review. I updated this PR.

kaicataldo
kaicataldo approved these changes Jan 9, 2020
View reviewed changes
Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

btmills
btmills approved these changes Jan 17, 2020
View reviewed changes
Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'm excited to start using this feature!

@btmills btmills merged commit cf46df7 into master Jan 17, 2020
v7.0.0 automation moved this from Implemented, pending review to Done Jan 17, 2020
@btmills btmills deleted the rfc33 branch January 17, 2020 15:58
@mysticatea mysticatea moved this from Implemented, pending review to Done in RFCs Jan 23, 2020
montmanu pushed a commit to montmanu/eslint that referenced this pull request Mar 4, 2020
@mysticatea @kaicataldo @montmanu
Breaking: description in directive comments (refs eslint/rfcs#33) (es…
58457c9 
…lint#12699)

* Breaking: description in directive comments (refs eslint/rfcs#33)

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update tests/lib/linter/linter.js

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* update configuring.md

Co-authored-by: Kai Cataldo <kai@kaicataldo.com>
@glen-84 glen-84 mentioned this pull request Jun 24, 2020
Closed
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jul 17, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@btmills btmills btmills approved these changes

@kaicataldo kaicataldo kaicataldo approved these changes

Assignees
No one assigned
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion breaking This change is backwards-incompatible core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
No open projects
RFCs
  
Done
v7.0.0
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mysticatea @btmills @kaicataldo