Skip to content

Add TargetPlatform linux/windows breaking change migration guide. - #3746

Merged
sfshaza2 merged 3 commits into
flutter:masterfrom
gspencergoog:target_platform
Mar 3, 2020
Merged

sfshaza2 merged 3 commits into
flutter:masterfrom
gspencergoog:target_platform

Conversation

@gspencergoog

Copy link
Copy Markdown
Contributor

This adds a migration guide to go with the flutter/flutter#51519 PR.

@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Feb 27, 2020

@sfshaza2 sfshaza2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from some grammar stuff, we can't merge this PR until we have an actual release #.

Comment thread src/docs/release/breaking-changes/target-platform-linux-windows.md Outdated
@@ -0,0 +1,153 @@
---
title: Adding `linux` and `windows` to [`TargetPlatform`][] enum
description: Two new values were added to the [`TargetPlatform`][] enum that could require additional cases in switch statements that switch on a [`TargetPlatform`][].

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The meta data shouldn't have any links. Remove both of these

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, done. Can the metadata lines be wrapped, or do they have to be one per line?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just seeing this question now! The description: line in the YML meta data needs to be a single line or Travis fails.

Comment thread src/docs/release/breaking-changes/target-platform-linux-windows.md Outdated
Comment thread src/docs/release/breaking-changes/target-platform-linux-windows.md Outdated
}
```

And the shim in `main()` is no longer required.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what you mean by "shim" but will other devs know?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably? I reworded it here and below to not use "shim" anyhow.

Comment thread src/docs/release/breaking-changes/target-platform-linux-windows.md Outdated
This can cause the Dart analyzer to give the [`missing_enum_constant_in_switch`][]
warning for switch statements which don't include a `default` case. Writing a
switch without a `default:` case is the recommended way to handle enums, since
the analyzer can then help you find any cases which aren't handled.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks for the guidance.

Comment thread src/docs/release/breaking-changes/target-platform-linux-windows.md Outdated

## Timeline

This change was made in March 2020.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have the release number? We can't merge this PR until we have an actual release #.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not yet. The PR is not yet committed. Once it is, I'll have a release number.

Relevant PRs:
* [Add Windows, and Linux as TargetPlatforms][]

[`debugDefaultTargetPlatformOverride`]: {{site.api}}/flutter/foundation/debugDefaultTargetPlatformOverride.html

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer these links to be alphabetical, but that's just a minor nit. I wouldn't hold up merging for that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The are alphabetical, it's just that the backtick character sorts before letters. :-)

Re-sorted, ignoring formatting marks.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

@sfshaza2, OK, I've added the specific version now that the PR has landed. Is there anything else this needs?

@sfshaza2 sfshaza2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sfshaza2
sfshaza2 merged commit 47def73 into flutter:master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Contributor has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants