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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce rule 7.9 in eslint (default-param-last) #2106

Open
wants to merge 2 commits into
base: master
from

Conversation

@vieira
Copy link

commented Oct 3, 2019

Enforces the existing airbnb style guide rule about default parameters coming last in eslint by using default-param-last.

As this rule was introduced in ESLint 6.4.0, I bumped the eslint version in package.json but I am not sure if that is the right thing to do? This is my first PR in this repository. 馃槰

@@ -57,7 +57,7 @@
"babel-preset-airbnb": "^4.0.1",
"babel-tape-runner": "^3.0.0",
"eclint": "^2.8.1",
"eslint": "^5.16.0 || ^6.1.0",
"eslint": "^5.16.0 || ^6.4.0",

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 3, 2019

Collaborator

this change would need to be made in both dev deps and peerDeps; it also is arguably a breaking change, but I've typically treated this as semver-minor in the past.

This comment has been minimized.

Copy link
@vieira

vieira Oct 3, 2019

Author

@ljharb Done, thanks for reviewing!

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Oct 3, 2019

I just realized that since this rule isn't in eslint 5, this would in fact be a breaking change - so it can't come in enabled just yet.

@vieira

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

Ahh I see, sorry, should I drop eslint 5 from the dependencies and you will merge this sometime in the future or is there other way of doing things?

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Oct 3, 2019

Typically I add all new rules as "off" as I update eslint versions, and leave a TODO comment to enable them - but they are not enabled until the next major release.

@vieira

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

@ljharb Understood, let me know if you need anything else from me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.