-
Notifications
You must be signed in to change notification settings - Fork 30k
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
debugger: revise async iterator usage to comply with lint rules #38847
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I guess... but it's pretty weird. Perhaps add a comment about this isn't just using for await
?
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: nodejs#38847 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in fc264df |
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: #38847 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
This doesn't land cleanly on v14.x-staging. If it shouldn't land, please adjust the labels accordingly. |
This will cherry-pick cleanly if you land #38811 first. Again, you'll have to be OK with landing primordials for the Cherry-picking in this order all lands cleanly on v14.x-staging as of this writing:
|
@Trott #38406 uses
While |
I guess our options are:
Did I miss anything? Which is the most desirable way to go with this? |
Actually, I think this one can just be not landed. It's a lint-only change and a small one at that. |
The last one, but that would need to wait for a semver-minor release of 14.x (I'm currently preparing a patch release). FYI @targos (as you've volunteered for the next 14.x semver-minor). |
That one can't really land on v14.x. The |
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: nodejs#38847 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
Backport to v14.x PR: #39446 |
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: nodejs#38847 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: nodejs#38847 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: #38847 Backport-PR-URL: #39446 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: #38847 Backport-PR-URL: #39446 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
I'm not sure that this is any clearer than the existing code, but I don't think it's significantly less clear, and it avoids comment disabling a lint rule. PR-URL: nodejs#38847 Backport-PR-URL: nodejs#39446 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]>
I'm not sure that this is any clearer than the existing code, but I
don't think it's significantly less clear, and it avoids comment
disabling a lint rule.