Prepare IsolateManager to become source of truth for isPaused for an isolate. #5005
Conversation
polina-c
changed the title
| return isolateState.isolate; | ||
| /// Resturns isolate by its reference. | ||
| /// | ||
| /// The method will return immediately in most cases |
There was a problem hiding this comment.
return immediately is misleading as the return value is a future.
This method name has always been bad. Some other ideas:
isolateFor(isolateRef)
isolate(isolateRef)
delete this method completely and just call
isolateState(isolateRef).isolate
everywhere this method was used.
| Future<bool> isIsolatePaused(IsolateRef ref) async { | ||
| final state = isolateState(ref); | ||
| await state.waitForIsolateToLoad(); | ||
| return state.isPaused.value!; |
There was a problem hiding this comment.
is value guaranteed to be non-null? should we have a default value instead?
packages/README.md
Outdated
| @@ -10,3 +10,11 @@ in compliance with [Flutter repo style guide]( https://github.com/flutter/flutte | |||
| 1. Public getter | |||
| 2. Private field | |||
| 3. Public setter (when needed) | |||
|
|
|||
| ## Naming for methods and functions | |||
There was a problem hiding this comment.
in general, I don't think we need to rewrite a style guide for our own repo. We can link to Effective Dart and the Flutter style guide for references from this README.MD or from out CONTRIBUTING.MD file (this may be a better place), but it seems a bit overkill to try to pick and choose some of the style guidelines we follow and rewrite them here.
There was a problem hiding this comment.
We do not follow the entire Flutter style guide (because it conflicts with Effective Dart or some rules do not make sense for us). But exactly these two rules make a lot of sense and will bring good consistency to our code. So, I suggest to link just them.
How does it sound?
Co-authored-by: Daniel Chevalier <danchevalier@google.com>
CONTRIBUTING.md
Outdated
| 2. Make sure your (or your organization's) name and contact info is added to the [AUTHORS](AUTHORS) | ||
| file. |
There was a problem hiding this comment.
I think we can remove this - we don't follow this practice. @jacob314 please correct me if I am wrong.
There was a problem hiding this comment.
Yeah I don't know of us ever following that practice. Does any repo under the Flutter org do this?
There was a problem hiding this comment.
Out of top 5 Flutter repos I checked only flutter-intellij has this instruction and follows the rule:
https://github.com/flutter/flutter-intellij/blob/master/CONTRIBUTING.md
https://github.com/flutter/flutter-intellij/blob/master/AUTHORS
RELEASE_NOTE_EXCEPTION=[refactoring]