Skip to content

Conversation

@cjihrig
Copy link
Contributor

This PR simplifies several of our custom ESLint rules by using selector syntax instead of handwritten functions to identify matching AST nodes.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

@nodejs-github-botnodejs-github-bot added the tools Issues and PRs related to the tools directory. label Dec 9, 2017
Copy link
Contributor

@apapirovskiapapirovski left a comment

Choose a reason for hiding this comment

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

Great stuff! Just a couple of nits for legibility but not critical :)

Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of legibility, we could extract this outside of module.exports and just use [astSelector]? Your call but I'm not a fan of the run-on line (having the [] beneath each other is a bit more legible IMHO).

Copy link
Contributor

Choose a reason for hiding this comment

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

This could be extracted into a function outside of module.exports that takes input for the argument position, so something like getAstSelector(pos). Then we also don't need to have the run on line.

@cjihrig
Copy link
ContributorAuthor

Pulled selectors out into variables, as requested.

CI: https://ci.nodejs.org/job/node-test-pull-request/12036/

@cjihrigcjihrig merged commit 746534b into nodejs:masterDec 12, 2017
@cjihrigcjihrig deleted the lint branch December 12, 2017 02:06
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@MylesBorinsMylesBorins mentioned this pull request Dec 12, 2017
@gibfahn
Copy link
Member

Missing landed in

Landed in e4a710966d3e6b0c1dee5746534b

gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@gibfahn
Copy link
Member

Didn't cherry-pick the last commit back to 6.x as it conflicted.

@gibfahngibfahn mentioned this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@gibfahngibfahn mentioned this pull request Dec 20, 2017
@MylesBorinsMylesBorins mentioned this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@cjihrig@gibfahn@apapirovski@MylesBorins@nodejs-github-bot