migration of material files to nullsafety #66633
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
There was a problem hiding this comment.
LGTM except for material_state.dart which I'd like @johnsonmh to review.
| @@ -224,15 +222,15 @@ abstract class MaterialStateMouseCursor extends MouseCursor implements MaterialS | |||
| @protected | |||
| @override | |||
| MouseCursorSession createSession(int device) { | |||
| return resolve(<MaterialState>{}).createSession(device); | |||
| return resolve(<MaterialState>{})!.createSession(device); | |||
There was a problem hiding this comment.
@dkwingsmt Can you take a look at this? In general resolve will return a nullable type, I worry that this ! isn't safe and should maybe look something more like:
return resolve(<MaterialState>{})?.createSession(device) ?? _defaultMouseCursor.createSession(device);There was a problem hiding this comment.
Yes. In most (all) cases, if MaterialState's resolve method returns null, that indicates that the widget should defer to the next most general default.
|
Could a googler check why Google testing is still Pending? |
|
We have some infra problem and cannot fetch this PR correctly. Looking into it now. |
|
This has something to do with the API we use. I will try to fix this by EOB Monday. |
Description
NNBD migration for part of material
Related Issues
Tests
I added the following tests:
None
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.