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

Suggestions for running the linter as part of landing process #484

Closed
mmarchini opened this issue Aug 18, 2020 · 2 comments
Closed

Suggestions for running the linter as part of landing process #484

mmarchini opened this issue Aug 18, 2020 · 2 comments

Comments

@mmarchini
Copy link
Contributor

While testing #473 I experienced landing with linter (which landed recently #435) and I have a couple suggestions:

  • Running the linter is surprisingly slow, which IMO makes the already heavyweight landing process even more cumbersome to do manually. Could we make the lint process opt-in, so folks can choose PRs they think will require lint before landing? We can set it on by default when landing via commit queue
  • After the lint finished running and failed, it was not clear what I should do. A message explaining that the user should fix lint issues and run --continue would be useful
  • The lint happens after applying the commits but before amending, which results in weird behavior (running --continue will amend and then stop, user will need to run --continue again to go to --final). I think linter should run as part of --final instead.
mmarchini added a commit to mmarchini/node-core-utils that referenced this issue Aug 20, 2020
@mmarchini
feat: make lint check opt-in
9ff409c 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs#484
@mmarchini mmarchini mentioned this issue Aug 20, 2020
Merged
mmarchini added a commit to mmarchini/node-core-utils that referenced this issue Aug 20, 2020
@mmarchini
feat: make lint check opt-in
595c4f8 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs#484
mmarchini added a commit that referenced this issue Aug 20, 2020
@mmarchini
feat: make lint check opt-in
b567c1e 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: #484
@github-actions
Copy link
Contributor

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Nov 17, 2020
@mmarchini mmarchini removed the stale label Nov 17, 2020
@github-actions
Copy link
Contributor

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Feb 16, 2021
@github-actions github-actions bot closed this as completed Mar 3, 2021
@targos targos removed the stale label Mar 3, 2021
johnfrench3 pushed a commit to johnfrench3/core-utils-node that referenced this issue Nov 2, 2022
feat: make lint check opt-in
cf179f4 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs/node-core-utils#484
renawolford6 added a commit to renawolford6/node-dev-build-core-utils that referenced this issue Nov 10, 2022
@renawolford6
feat: make lint check opt-in
1ed457e 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs/node-core-utils#484
Developerarif2 pushed a commit to Developerarif2/node-core-utils that referenced this issue Jan 27, 2023
@logictitans
feat: make lint check opt-in
b4dae4a 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs/node-core-utils#484
gerkai added a commit to gerkai/node-core-utils-project-build that referenced this issue Jan 27, 2023
@gerkai
feat: make lint check opt-in
fd38f22 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs/node-core-utils#484
patrickm68 added a commit to patrickm68/NodeJS-core-utils that referenced this issue Sep 14, 2023
@patrickm68
feat: make lint check opt-in
3d41ebd 
Disable lint check by default and make it opt-in, so it doesn't impact
the workflow for collaborators, while providing a way for early birds to
try it out so we can tweak it until it works great for everyone (at
which point we can re-enable it by default).

Ref: nodejs/node-core-utils#484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@targos @mmarchini