-
Notifications
You must be signed in to change notification settings - Fork 26.6k
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
Added max-len rule #510
base: master
Are you sure you want to change the base?
Added max-len rule #510
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,7 @@ Other Style Guides | |
1. [Accessors](#accessors) | ||
1. [Events](#events) | ||
1. [jQuery](#jquery) | ||
1. [Line length](#line-length) | ||
1. [ECMAScript 5 Compatibility](#ecmascript-5-compatibility) | ||
1. [ECMAScript 6 Styles](#ecmascript-6-styles) | ||
1. [Testing](#testing) | ||
|
@@ -1986,16 +1987,35 @@ Other Style Guides | |
|
||
**[⬆ back to top](#table-of-contents)** | ||
|
||
## Line length | ||
|
||
- [26.1](#26.1) <a name='26.1'></a> Avoid very long lines of code longer than 100 characters. | ||
|
||
> Why? This ensures readability and maintainability. | ||
|
||
```javascript | ||
// bad | ||
var foo = { bar: "This is a bar.", baz: { "qux": "This is a qux" }, difficult: "to read" }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "qux" is unnecessarily quoted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this line should be obviously too long, as it is it's 96 characters, make it like 120+ chars There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I am sorry. This happens when you are in hurry 😕. I will fix that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. re ordering these is :( because it will break existing links into the styleguide There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm so the solution is? Append the new entry to the bottom of the document? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are existing links into the styleguide using numbered sections instead of named ones? That seems like something we should fix regardless. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I support this idea. We already have named links for ES6 styles. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Named sections++ from the C++ Core Guidelines:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add named sections to my doc generator in #480 |
||
|
||
// good | ||
var foo = { | ||
bar: "This is a bar.", | ||
baz: { | ||
qux: "This is a qux" | ||
}, | ||
difficult: "to read" | ||
}; | ||
``` | ||
|
||
## ECMAScript 5 Compatibility | ||
|
||
- [26.1](#26.1) <a name='26.1'></a> Refer to [Kangax](https://twitter.com/kangax/)'s ES5 [compatibility table](http://kangax.github.com/es5-compat-table/). | ||
- [27.1](#27.1) <a name='26.1'></a> Refer to [Kangax](https://twitter.com/kangax/)'s ES5 [compatibility table](http://kangax.github.com/es5-compat-table/). | ||
|
||
**[⬆ back to top](#table-of-contents)** | ||
|
||
## ECMAScript 6 Styles | ||
|
||
- [27.1](#27.1) <a name='27.1'></a> This is a collection of links to the various es6 features. | ||
- [28.1](#28.1) <a name='27.1'></a> This is a collection of links to the various es6 features. | ||
|
||
1. [Arrow Functions](#arrow-functions) | ||
1. [Classes](#constructors) | ||
|
@@ -2015,7 +2035,7 @@ Other Style Guides | |
|
||
## Testing | ||
|
||
- [28.1](#28.1) <a name='28.1'></a> **Yup.** | ||
- [29.1](#29.1) <a name='28.1'></a> **Yup.** | ||
|
||
```javascript | ||
function() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,6 +83,7 @@ module.exports = { | |
}], | ||
'eqeqeq': 2, // http://eslint.org/docs/rules/eqeqeq | ||
'guard-for-in': 2, // http://eslint.org/docs/rules/guard-for-in | ||
'max-len': [2, 100, 4] // http://eslint.org/docs/rules/max-len | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comma missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, |
||
'no-caller': 2, // http://eslint.org/docs/rules/no-caller | ||
'no-else-return': 2, // http://eslint.org/docs/rules/no-else-return | ||
'no-eq-null': 2, // http://eslint.org/docs/rules/no-eq-null | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding new section, perhaps you can add your new rule to the
Whitespace
sectionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm do you think that this should really be in the
Whitespace
section?