Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented May 28, 2019

Description

Based on discussion from last week, returns a list of strings containing each possible platform supported, independent of current devices or workflows.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a 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 :)

@goderbauer goderbauer added the tool Affects the "flutter" command-line tool. See also t: labels. label May 28, 2019
if (flutterProject.linux.existsSync()) {
result.add('linux');
}
if (flutterProject.macos.existsSync()) {
Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor

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) 👍

@jonahwilliams jonahwilliams requested a review from dnfield May 30, 2019 16:37
return <String, Object>{
'platforms': result,
};
} catch (err, stackTrace) {
Copy link
Contributor

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....

Copy link
Contributor

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.

@jonahwilliams jonahwilliams merged commit f38ee15 into flutter:master May 30, 2019
@jonahwilliams jonahwilliams deleted the projects_per_version branch May 30, 2019 17:14
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants