Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[ios_platform_images] migrate objC to swift #4847
[ios_platform_images] migrate objC to swift #4847
Changes from 12 commits
ad9ed35
e5545bc
cf21182
008d5d3
e422e78
b0122cf
54b9b92
6c29139
f3ce031
100202a
97dc083
1a28d20
8da12bd
50ce448
59d7b6d
17a369e
cfbf55f
101c190
1d2969b
372ce11
523a277
aef7985
78cbc82
45d185d
a9a4879
364be83
aae7a29
cbbc208
ac54592
3e66718
71f2589
e2d9a9a
3324ccd
e370d61
b25d574
5d03976
eaf7c90
139e125
6ea1fc0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
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.
Flutter style prefers not using timeouts in tests, as they contribute to flake. This should just wait until the expectation is fulfilled.
Same in all of the tests below.
This file was deleted.
This file was deleted.
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.
What's the purpose of the intermediate
imageResult
variable, rather than inlining the result?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.
As above, this should not fail silently.
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 is not the same logic as the Obj-C version, and is not correct. There will always be two arguments, but the second may be
NSNull
.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.
I change to avoid
Fatal error: Index out of range
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.
In what context? This is the call, and it passes two arguments.
If you are referring to the unit tests you added, the solution is to make the unit tests match actual usage. Having unit tests that call the method in a way that production code never will, and then changing the implementation to work in the unit test but break production, makes the unit tests actively harmful.
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.
I saw here in another test the reason for your comment, I will make the appropriate change
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.
Why the explicit branching, rather than optional chaining like the Obj-C version? The more
result
calls there are, the harder it is to make sure the logic is correctly calling it exactly one time.This file was deleted.