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

cluster: migrate round_robin_handle to internal assert #26047

Closed
wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Feb 11, 2019

Use smaller internal assert module for round_robin_handle.js.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the cluster Issues and PRs related to the cluster subsystem. label Feb 11, 2019
@Trott
Copy link
Member Author

Trott commented Feb 11, 2019

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 12, 2019
@Trott
Copy link
Member Author

Trott commented Feb 12, 2019

@Trott
Copy link
Member Author

Trott commented Feb 12, 2019

@Trott Trott force-pushed the internal-assert-for-round-robin branch from f3a03ff to 8491298 Compare February 13, 2019 00:47
@Trott
Copy link
Member Author

Trott commented Feb 13, 2019

OK, this now adds a tiny assert.fail() to the internal module and uses that instead so as to avoid pitfalls and confusion.

PTAL. Thanks.

Pinging people who approved previously for a re-review: @cjihrig @antsmartian @jasnell @lpinca

And pinging folks who had comments that I believe this change now addresses: @mscdex @bnoordhuis

@Trott
Copy link
Member Author

Trott commented Feb 13, 2019

@Trott Trott removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 13, 2019
@lpinca
Copy link
Member

lpinca commented Feb 13, 2019

Still LGTM.

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 13, 2019
@addaleax
Copy link
Member

Landed in ce65034, 4aa04f5

@addaleax addaleax closed this Feb 13, 2019
addaleax pushed a commit that referenced this pull request Feb 13, 2019
PR-URL: #26047
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Feb 13, 2019
Use smaller internal assert module for round_robin_handle.js.

PR-URL: #26047
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Feb 14, 2019
PR-URL: #26047
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Feb 14, 2019
Use smaller internal assert module for round_robin_handle.js.

PR-URL: #26047
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Feb 14, 2019
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. cluster Issues and PRs related to the cluster subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants