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

test: replace console.log/error with debuglog #32695

Closed
wants to merge 1 commit into from

Conversation

agustindaguerre
Copy link
Contributor

@agustindaguerre agustindaguerre commented Apr 6, 2020

Use utility debug logs instead of console logs in test-cluster-setup-master-multiple.js

Refs: #32678

Checklist

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 6, 2020
@jasnell
Copy link
Member

jasnell commented Apr 6, 2020

Just a nit... the commit should use Refs: instead of Fixes: in referencing the original issue so that the issue does not get closed when this one PR lands.

@agustindaguerre
Copy link
Contributor Author

@jasnell updated 👍

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@himself65 himself65 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 9, 2020
@himself65

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@himself65
Copy link
Member

I'm sure this PR is ok but, I don't now why CI failed so many times(I'm a newbie on Jenkins CI) 🤔

@nodejs-github-bot

This comment has been minimized.

Use utility debug logs instead of console logs in test-cluster-setup-master-multiple.js

Refs: nodejs#32678
@nodejs-github-bot
Copy link
Collaborator

@himself65 himself65 self-assigned this Apr 9, 2020
@himself65
Copy link
Member

I will handle merging this

@agustindaguerre
Copy link
Contributor Author

@himself65 cool, thanks

himself65 pushed a commit that referenced this pull request Apr 9, 2020
Use utility debug logs instead of console logs
in test-cluster-setup-master-multiple.js

Refs: #32678

PR-URL: #32695
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
@himself65
Copy link
Member

Landed in 41b1e87

Thanks for your contributions!🎉

@himself65 himself65 closed this Apr 9, 2020
@himself65
Copy link
Member

himself65 commented Apr 9, 2020

BTW, the original commit message is invalid because

  ✖  da7d991c0221b662bf56a8d4861353237b84047e
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✖  1:72     Line should be <= 72 columns.             line-length
     ✔  0:0      metadata is at end of message             metadata-end
     ✔  5:8      PR-URL is valid.                          pr-url
     ✔  0:0      reviewers are valid                       reviewers
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ✔  0:0      Title is <= 50 columns.                   title-length

so I edit it manually

targos pushed a commit that referenced this pull request Apr 12, 2020
Use utility debug logs instead of console logs
in test-cluster-setup-master-multiple.js

Refs: #32678

PR-URL: #32695
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 14, 2020
Use utility debug logs instead of console logs
in test-cluster-setup-master-multiple.js

Refs: #32678

PR-URL: #32695
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
targos pushed a commit that referenced this pull request Apr 22, 2020
Use utility debug logs instead of console logs
in test-cluster-setup-master-multiple.js

Refs: #32678

PR-URL: #32695
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants