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.

Sign up for GitHub

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

feat: Support ESLint 7.x #30

Merged
merged 8 commits into from
May 12, 2020
Merged

feat: Support ESLint 7.x #30

merged 8 commits into from
May 12, 2020

Conversation

MichaelDeBoey
Copy link
Contributor

@MichaelDeBoey MichaelDeBoey commented May 8, 2020 •
edited
Loading

ESLint v7.0.0 is released 🎉

(dev)Dependencies should be compatible with ESLint 7 too before we can merge this one:

Closes #29

@MichaelDeBoey MichaelDeBoey mentioned this pull request May 8, 2020
Draft
9 tasks
@MichaelDeBoey
Copy link
Contributor Author

@mysticatea If you can point me into the right direction for what's wrong, I'm happy to make CI pass 🙂

@mysticatea
Copy link
Owner

Thank you for your contribution.

  1. Please don't make breaking changes. Dropping old nodejs support is not required.
  2. The values of node.start and node.range[0] can be different. In this case, the node.start of TemplateElement nodes is wrong -- it doesn't contain backtick. On the other hand, the node.range[0] of TemplateElement nodes is correct. You have to address it.

@MichaelDeBoey MichaelDeBoey force-pushed the eslint-7 branch 2 times, most recently from 986ce64 to 00ecd38 Compare May 10, 2020 12:21
@MichaelDeBoey
Copy link
Contributor Author

@mysticatea I've updated tests to pass CI.
Only 2 tests are still failing, but don't know what to do with those.

Hopefully you can help me into the right direction on this one 🙂

@mysticatea
Copy link
Owner

Hi, please don't change tests. See #30 (comment)

@MichaelDeBoey
Copy link
Contributor Author

@mysticatea I think I'm going into the right direction, but still have some failures.

@mysticatea
Copy link
Owner

The problem is, I guess, the range of text and the range of node are different. Using sourceCode.getText(node) at all may solve.

mysticatea
mysticatea requested changes May 10, 2020
View reviewed changes
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@MichaelDeBoey MichaelDeBoey requested a review from mysticatea May 10, 2020 13:38
mysticatea
mysticatea approved these changes May 10, 2020
View reviewed changes
Copy link
Owner

@mysticatea mysticatea left a comment

Choose a reason for hiding this comment

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

LGTM, thank you very much!
Currently I'm on mobile, so I will merge this later.

@MichaelDeBoey
Copy link
Contributor Author

Would be awesome to get this one merged 👊

Happy to help out on the other ones too 🙂

@mysticatea mysticatea merged commit e147de9 into mysticatea:master May 12, 2020
@MichaelDeBoey MichaelDeBoey deleted the eslint-7 branch May 12, 2020 11:03
@MichaelDeBoey
Copy link
Contributor Author

@mysticatea Any idea when this will be released? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mysticatea mysticatea mysticatea approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support ESLint 7.x
2 participants
@MichaelDeBoey @mysticatea