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

[2.19] Concurrency updates for isolate.run() #4357

Merged
merged 13 commits into from
Dec 20, 2022
Merged

[2.19] Concurrency updates for isolate.run() #4357

merged 13 commits into from
Dec 20, 2022

Conversation

MaryaBelanger
Copy link
Contributor

@MaryaBelanger MaryaBelanger commented Nov 8, 2022 •
edited
Loading

Figuring out code excerpt stuff before review

Closes #3904

Staged here: https://dart-dev-marya.web.app/guides/language/concurrency
Specifically the Code examples section

@MaryaBelanger MaryaBelanger self-assigned this Nov 8, 2022
lrhn
lrhn reviewed Nov 9, 2022
View reviewed changes
@atsansone atsansone self-requested a review November 9, 2022 19:48
@atsansone atsansone self-assigned this Nov 9, 2022
@MaryaBelanger MaryaBelanger requested a review from lrhn November 10, 2022 19:02
@MaryaBelanger
Copy link
Contributor Author

Checks failing because Isolate.run() is undefined.
Will stay that way until 2.19 release.

@parlough
Copy link
Member

parlough commented Nov 10, 2022 •
edited
Loading

Can you also run dart format on the updated .dart files and refresh code excerpts? That's what's causing beta and dev to fail.

@parlough parlough requested a review from johnpryan November 10, 2022 20:01
@MaryaBelanger
Copy link
Contributor Author

Can you also run dart format on the updated .dart files and refresh code excerpts? That's what's causing beta and dev to fail.

Oh, nice! It was concurrency_test.dart that I missed, main() needed a Future

atsansone
atsansone suggested changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@atsansone atsansone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaryaBelanger : There's a few changes here along the lines of agency, conciseness, and direct language. Happy to discuss.

johnpryan
johnpryan reviewed Nov 10, 2022
View reviewed changes
Copy link
Contributor

@johnpryan johnpryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good overall, Isolate.run() is much easier to use 👍

@MaryaBelanger MaryaBelanger mentioned this pull request Nov 14, 2022
Closed
@MaryaBelanger MaryaBelanger changed the title Concurrency updates for isolate.run() [2.19] Concurrency updates for isolate.run() Nov 29, 2022
@MaryaBelanger MaryaBelanger marked this pull request as ready for review December 5, 2022 23:38
atsansone
atsansone approved these changes Dec 9, 2022
View reviewed changes
Copy link
Contributor

@atsansone atsansone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tight copy.

@parlough parlough self-requested a review December 9, 2022 20:44
@MaryaBelanger
Copy link
Contributor Author

@parlough could you help with / explain the failing check? I scanned the details but don't see exactly what the issue is

@parlough
Copy link
Member

parlough commented Dec 13, 2022 •
edited
Loading

@MaryaBelanger Hmm, the message doesn't show it well, but since Isolate.run doesn't exist, the tests for examples/concurrency will fail to run on stable. For these changes you just need to refer to the beta tests.

I guess we can maybe configure it to show the underlying compiler error, but usually analysis will catch any issues anyway.

@MaryaBelanger MaryaBelanger changed the base branch from main to v2.19 December 14, 2022 18:57
@atsansone atsansone added this to the Next Minor Release milestone Dec 20, 2022
@atsansone atsansone merged commit a925157 into v2.19 Dec 20, 2022
parlough added a commit that referenced this pull request Jan 23, 2023
@MaryaBelanger @parlough
[2.19] Concurrency updates for isolate.run() (#4357)
7af7609 
Figuring out code excerpt stuff before review

Closes #3904 

Staged here: https://dart-dev-marya.web.app/guides/language/concurrency
Specifically the [Code
examples](https://dart-dev-marya.web.app/guides/language/concurrency#code-examples)
section

Co-authored-by: Parker Lougheed <parlough@gmail.com>
parlough added a commit that referenced this pull request Jan 23, 2023
@MaryaBelanger @parlough
[2.19] Concurrency updates for isolate.run() (#4357)
6c68110 
Updates 'Concurrency in Dart' page for `Isolate.run`.

Closes #3904

Co-authored-by: Parker Lougheed <parlough@gmail.com>
parlough added a commit that referenced this pull request Jan 24, 2023
@MaryaBelanger @parlough
[2.19] Concurrency updates for isolate.run() (#4357)
107f1f6 
Updates 'Concurrency in Dart' page for `Isolate.run`.

Closes #3904

Co-authored-by: Parker Lougheed <parlough@gmail.com>
parlough added a commit that referenced this pull request Jan 24, 2023
@MaryaBelanger @parlough
[2.19] Concurrency updates for isolate.run() (#4357)
187be61 
Updates 'Concurrency in Dart' page for `Isolate.run`.

Closes #3904

Co-authored-by: Parker Lougheed <parlough@gmail.com>
@parlough parlough deleted the feature/isolate.run branch January 24, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atsansone atsansone atsansone approved these changes

@johnpryan johnpryan johnpryan approved these changes

@lrhn lrhn Awaiting requested review from lrhn

@parlough parlough Awaiting requested review from parlough

Assignees

@atsansone atsansone

@MaryaBelanger MaryaBelanger

Labels
None yet
Projects
None yet
Milestone
Next Minor Release
Development

Successfully merging this pull request may close these issues.

[2.19] Isolate.run() docs
5 participants
@MaryaBelanger @parlough @atsansone @johnpryan @lrhn