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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linter: add max-len rules #507

Merged
merged 5 commits into from
Jun 19, 2019
Merged

linter: add max-len rules #507

merged 5 commits into from
Jun 19, 2019

Conversation

axetroy
Copy link
Contributor

@axetroy axetroy commented Jun 18, 2019

If strict execution of each line has a maximum width of 80 characters

Then there will be a lot of code getting ugly.

There are 3 lines below, which are 80/100/120 characters.

屏幕快照 2019-06-19 00 09 02

We can set the options to ignore comments/string and so on.

Just wrap code lines at 80 characters.

@ry
Copy link
Member

ry commented Jun 18, 2019

I prefer strict 80

.eslintrc.json Outdated
{
"code": 80,
"tabWidth": 2,
"comments": 200,
Copy link
Member

Choose a reason for hiding this comment

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

200 columns for comments? that’s not ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just an example.

so, all strict to 80. including comments?

Copy link
Contributor Author

@axetroy axetroy Jun 18, 2019

Choose a reason for hiding this comment

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

There is some open source license that is significantly more than 80 characters.

This will modify these license to fix the error.

that's not cool.

屏幕快照 2019-06-19 00 25 38

Copy link
Member

@ry ry Jun 18, 2019

Choose a reason for hiding this comment

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

Yes, including comments (and licenses).

It seems you have a very wide monitor, but I program exclusively on a 12 inch laptop. So here is how it looks on my machine (full screen terminal):

Screen Shot 2019-06-18 at 12 59 05 PM

@ry
Copy link
Member

ry commented Jun 18, 2019

Thanks for doing this!

@axetroy axetroy marked this pull request as ready for review June 19, 2019 04:15
@axetroy
Copy link
Contributor Author

axetroy commented Jun 19, 2019

/cc @ry

it's ready for review.

Currently, only the length of the URL is ignored.

      {
        "code": 80,
        "tabWidth": 2,
        "comments": 80,
        "ignoreTrailingComments": false,
        "ignoreUrls": true,
        "ignoreStrings": false,
        "ignoreTemplateLiterals": false,
        "ignoreRegExpLiterals": false
      }

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM
Very nice ! Thanks!

@ry ry merged commit b04fda3 into denoland:master Jun 19, 2019
@axetroy axetroy deleted the eslint_max_len branch June 19, 2019 04:22
ry pushed a commit to ry/deno that referenced this pull request Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants