FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tools: disable max-len ESLint rule for strings by Trott · Pull Request #41536 · nodejs/node · GitHub

/ node Public

tools: disable max-len ESLint rule for strings - #41536

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:eslint-max-len-strings
ConversationCommits (1)ChecksFiles changed
Closed

tools: disable max-len ESLint rule for strings#41536
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:eslint-max-len-strings

Conversation

Trott commented Jan 15, 2022

Copy link
Copy Markdown
Member

No description provided.

nodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jan 15, 2022
Trott mentioned this pull request Jan 15, 2022
Closed
VoltrexKeyva added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Jan 15, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 15, 2022

nodejs-github-bot commented Jan 15, 2022

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 17, 2022

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Jan 17, 2022

Copy link
Copy Markdown
Collaborator

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

jasnell requested changes Jan 17, 2022
View reviewed changes

jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd like to have more discussion on this. I'm not a fan of removing max line lengths. I'm happy to increase the current limits but not so much removing them entirely. I know this PR is limiting that change to just strings but still, I'd like more discussion around this first.

github-actions Bot mentioned this pull request Jan 18, 2022
Open
16 tasks
aduh95 removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jan 18, 2022
github-actions Bot mentioned this pull request Jan 19, 2022
Open
13 tasks

tniessen commented Jan 19, 2022

Copy link
Copy Markdown
Member

Alternative: #41509
Alternative: #41586

tniessen mentioned this pull request Jan 19, 2022
Merged
mcollina approved these changes Jan 19, 2022
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

Trott commented Jan 19, 2022

Copy link
Copy Markdown
Member Author

I'm happy to increase the current limits but not so much removing them entirely.

#41586

targos commented Jan 24, 2022

Copy link
Copy Markdown
Member

I'm still in favor of this change after #41586

Trott closed this Apr 6, 2022
Trott deleted the eslint-max-len-strings branch September 25, 2022 17:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell requested changes
mcollina mcollina approved these changes
benjamingr benjamingr approved these changes
himself65 himself65 approved these changes
+2 more reviewers
Mesteery Mesteery approved these changes
VoltrexKeyva VoltrexKeyva approved these changes
Reviewers whose approvals may not affect merge requirements

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.

Back | FazBrowse Home | New Git URL