-
Notifications
You must be signed in to change notification settings - Fork 29.7k
add daemon command to enumerate supported platforms #33472
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
add daemon command to enumerate supported platforms #33472
Conversation
stuartmorgan-g
left a comment
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.
One comment; I'll leave the real review to people who will actually be using the API :)
| if (flutterProject.linux.existsSync()) { | ||
| result.add('linux'); | ||
| } | ||
| if (flutterProject.macos.existsSync()) { |
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.
Should we also check the platform we're running on, so if someone commits a macos folder it doesn't cause it to show up as a target if you clone and try to run on Windows?
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 that case the project supports macos, but no device would support windows, so the filtering would happen on the device side.
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.
Ah yeah, that makes sense (and would match if we have metadata - you might load a project that claims it can target macOS but you're running on Windows) 👍
| return <String, Object>{ | ||
| 'platforms': result, | ||
| }; | ||
| } catch (err, stackTrace) { |
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.
Can we target a specific exception here? We should avoid generalized catches....
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 see now - we want the consumer get the exception and not crash. Ok.
Description
Based on discussion from last week, returns a list of strings containing each possible platform supported, independent of current devices or workflows.