-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Tiny code cleanup: Remove _asyncRunZoned
being an alias of runZoned
#52778
Conversation
Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at: https://dart-review.googlesource.com/c/sdk/+/311157 Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly. Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR). |
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
2 similar comments
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
sdk/lib/_http/overrides.dart
Outdated
@@ -54,7 +52,7 @@ abstract class HttpOverrides { | |||
findProxyFromEnvironment}) { | |||
HttpOverrides overrides = | |||
_HttpOverridesScope(createHttpClient, findProxyFromEnvironment); | |||
return _asyncRunZoned<R>(body, | |||
return runZoned<R>(body, |
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.
This change makes the runZoned
function of the HttpOverrides
class call itself, instead of calling the same-named function from dart:async
.
Most likely leading to a stack overflow.
Do that's why we have the rename, to be able to refer to the original, even though it's shadowed by a declaration with the same name.
(I'd recommend using an import with a prefix instead, though, to avoid creating the closure.)
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.
Ah I see, thank you! (Originally wanted to play with Dart code review system because I may submit serious PRs in the future so want to get familiar with it).
(I will get back and check CI soon) |
_asyncRunZoned
being an alias of runZoned
_asyncRunZoned
being an alias of runZoned
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
4 similar comments
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
https://dart-review.googlesource.com/c/sdk/+/311157 has been updated with the latest commits from this pull request. |
Sorry, I didn't let you wait. The code was fine in Gerrit, and I ran the presubmit successfully, so I accepted it there. |
@lrhn Thank you, and hope I can submit real PRs to Dart later! |
Looking forward to it. |
This is just super tiny code cleanup. I am not sure why we have:
when reading the IOOverrides code. IMHO, runZoned already handles the async, and everyone using it should be aware of that (at least Dart SDK developers who are very familiar with Dart). In addition, adding such alias will not change the behavior of the function.
Contribution guidelines:
dart format
.Note that this repository uses Gerrit for code reviews. Your pull request will be automatically converted into a Gerrit CL and a link to the CL written into this PR. The review will happen on Gerrit but you can also push additional commits to this PR to update the code review.