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

Chore: Mark SourceCode getComments() method as deprecated (fixes #13293) #13296

Merged
merged 1 commit into from May 19, 2020
Merged

Chore: Mark SourceCode getComments() method as deprecated (fixes #13293) #13296

merged 1 commit into from May 19, 2020

Conversation

SuperOleg39
Copy link
Contributor

@SuperOleg39 SuperOleg39 commented May 13, 2020 •
edited by mdjermanovic

Prerequisites checklist

  • I have read the contributing guidelines.
  • The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

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

[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Update source-code.js getComments method:

Program node exclude leading and trailing comments from our range, and i add special conditions, when given node has parent Program, leading and trailing comments tokens always belong to this node

@jsf-clabot
Copy link

jsf-clabot commented May 13, 2020 •
edited

CLA assistant check
All committers have signed the CLA.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label May 13, 2020
@SuperOleg39 SuperOleg39 changed the title Fix: Change SourceCode usage deprecated node properties start and end… Fix: Change SourceCode usage deprecated node properties start and end to range (fixes #13293) May 13, 2020
@SuperOleg39 SuperOleg39 changed the title Fix: Change SourceCode usage deprecated node properties start and end to range (fixes #13293) Fix: Change SourceCode usage deprecated node properties (fixes #13293) May 13, 2020
kaicataldo
kaicataldo suggested changes May 13, 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.

Given that SouceCode#getComments() is deprecated, we won't be accepting any more PRs to change the behavior. A PR updating the JSDoc block to include an @deprecated tag would be welcome, though!

@kaicataldo kaicataldo added works as intended The behavior described in this issue is working correctly and removed triage An ESLint team member will look at this issue soon labels May 13, 2020
@kaicataldo kaicataldo mentioned this pull request May 13, 2020
Closed
@SuperOleg39 SuperOleg39 changed the title Fix: Change SourceCode usage deprecated node properties (fixes #13293) Docs: Mark SourceCode getComments() method as deprecated (fixes #13293) May 14, 2020
@SuperOleg39
Copy link
Contributor Author

Given that SouceCode#getComments() is deprecated, we won't be accepting any more PRs to change the behavior. A PR updating the JSDoc block to include an @deprecated tag would be welcome, though!

Thank you, update PR!

kaicataldo
kaicataldo reviewed May 15, 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.

Can we change the commit message from Docs: to Chore:? We reserve Docs: for publicly visible documentation.

Otherwise, this LGTM!

@SuperOleg39 SuperOleg39 changed the title Docs: Mark SourceCode getComments() method as deprecated (fixes #13293) Chore: Mark SourceCode getComments() method as deprecated (fixes #13293) May 18, 2020
@SuperOleg39
Copy link
Contributor Author

Done!

@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion chore This change is not user-facing and removed works as intended The behavior described in this issue is working correctly labels May 18, 2020
mdjermanovic
mdjermanovic approved these changes May 18, 2020
View reviewed changes
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@SuperOleg39
Copy link
Contributor Author

☺️

@kaicataldo
Copy link
Member

LGTM, thank you!

@kaicataldo kaicataldo merged commit 72a4e10 into eslint:master May 19, 2020
@SuperOleg39 SuperOleg39 deleted the issue13293 branch May 20, 2020 08:00
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Nov 16, 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 Nov 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kaicataldo kaicataldo kaicataldo approved these changes

@mdjermanovic mdjermanovic mdjermanovic 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 chore This change is not user-facing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@SuperOleg39 @jsf-clabot @kaicataldo @mdjermanovic