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

tools: use error instead of 2 #14061

Merged
merged 1 commit into from
Jul 5, 2017
Merged

tools: use error instead of 2 #14061

merged 1 commit into from
Jul 5, 2017

Conversation

refack
Copy link
Contributor

@refack refack commented Jul 3, 2017

We are already using the "cleaner" YAML variant, so why not use the error symbol instead of the quite confusing 2 value?

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Jul 3, 2017
benjamingr
benjamingr approved these changes Jul 3, 2017
View reviewed changes
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

+1 for change, content LGTM

@refack
Copy link
Contributor Author

refack commented Jul 3, 2017

Linter: https://ci.nodejs.org/job/node-test-linter/10242/

@refack refack self-assigned this Jul 3, 2017
@vsemozhetbyt
Copy link
Contributor

What about:
https://github.com/nodejs/node/blob/master/benchmark/.eslintrc.yaml
https://github.com/nodejs/node/blob/master/doc/.eslintrc.yaml
https://github.com/nodejs/node/blob/master/lib/.eslintrc.yaml
https://github.com/nodejs/node/blob/master/test/.eslintrc.yaml
https://github.com/nodejs/node/blob/master/tools/.eslintrc.yaml

@refack
Copy link
Contributor Author

refack commented Jul 3, 2017

new Linter: https://ci.nodejs.org/job/node-test-linter/10244/

@refack
tools: eslint - use error and off
46d7cb8 
PR-URL: nodejs#14061
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@refack refack merged commit 46d7cb8 into nodejs:master Jul 5, 2017
@refack refack deleted the eslint-rules-2-to-error branch July 5, 2017 22:24
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@refack @addaleax
tools: eslint - use error and off
79493e1 
PR-URL: #14061
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@addaleax addaleax mentioned this pull request Jul 11, 2017
Merged
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@refack @addaleax
tools: eslint - use error and off
7d7da98 
PR-URL: #14061
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
@refack @Fishrock123
tools: eslint - use error and off
20c7b58 
PR-URL: #14061
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

@refack refack mentioned this pull request Aug 15, 2017
Closed
2 tasks
@refack
Copy link
Contributor Author

refack commented Aug 15, 2017

Backport PR: #14841

refack added a commit to refack/node that referenced this pull request Sep 19, 2017
@refack
tools: eslint - use error and off
328f04c 
PR-URL: nodejs#14061
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

# Conflicts:
#	.eslintrc.yaml
#	benchmark/.eslintrc.yaml
#	doc/.eslintrc.yaml
#	test/.eslintrc.yaml
#	tools/.eslintrc.yaml
MylesBorins pushed a commit that referenced this pull request Sep 19, 2017
@refack @MylesBorins
tools: eslint - use error and off
f7b6d19 
Backport-PR-URL: #14841
PR-URL: #14061
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
Merged
@refack refack removed their assignment Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fhinkel fhinkel fhinkel approved these changes

@benjamingr benjamingr benjamingr approved these changes

@lpinca lpinca lpinca approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@vsemozhetbyt vsemozhetbyt vsemozhetbyt approved these changes

Assignees
No one assigned
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@refack @vsemozhetbyt @MylesBorins @fhinkel @benjamingr @lpinca @targos @cjihrig @BridgeAR @nodejs-github-bot