Move target devices logic to its own classes and file #121903
Conversation
flutter-dashboard
bot
added
a: text input
| }) async { } | ||
| } | ||
|
|
||
| class FakeDeviceManager extends Fake implements DeviceManager { |
There was a problem hiding this comment.
I removed this so it uses the FakeDeviceManager in context.dart instead.
vashworth
changed the title
| return devices.where((Device device) { | ||
| return device.id == deviceId || device.id.startsWith(deviceId); | ||
| }).toList(); |
There was a problem hiding this comment.
Did you mean to change this logic in this refactoring PR?
There was a problem hiding this comment.
Yes, it was needed for this test.
In the old version of this test, findAllTargetDevices basically mocked the returned device list so it always returned the device list given. So the mocked getDevicesById was never actually called.
|
@jmagman I made an adjustment - moving the logic that prioritizes ephemeral devices to the DeviceManager to make it more accessible for a g3fix |
| List<Device> prioritizeEphemeralDevices(List<Device> devices) { | ||
| return devices; | ||
| } |
There was a problem hiding this comment.
I've noticed a few places in your code in previous PRs where you could have used =>, not sure if you're aware of that arrow dart syntax (this isn't even a nit comment, you don't have to change this).
| /// | ||
| /// Note: ephemeral is nullable for device types where this is not well | ||
| /// defined. | ||
| List<Device> prioritizeEphemeralDevices(List<Device> devices){ |
There was a problem hiding this comment.
This method name sounds like it should return a bool, like whether it's prioritized, or that it returns a list of the devices with the ephemeral devices sorted first.
How about filterToSingleEphemeralDeviceIfNeeded (or something better than that, it's clunky)?
There was a problem hiding this comment.
What do you think of getSoleEphemeralDevice and change return type to Device? and return null if not found?
There was a problem hiding this comment.
I like the Device? pattern better. How about getSingleEphemeralDevice instead of "sole" since "single" has a meaning in dart (though we def don't need to throw the StateError: https://api.dart.dev/be/180791/dart-core/Iterable/single.html)
vashworth
added
the
autosubmit
3 participants
This PR moves some of the logic involved in finding target devices into its own class and file. Reason for this is in future updates the logic will be more complex and there will be a Mac-specific class that inherits from the TargetDevices class. It also moves some logic from the
flutter devicescommand to its own class for the same reason.Part 2 in breakdown of #121262.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.