Skip to content

Create migration-guide.md #3

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

Merged
merged 5 commits into from
Nov 7, 2023
Merged

Create migration-guide.md #3

merged 5 commits into from
Nov 7, 2023

Conversation

mit-mit
Copy link

@mit-mit mit-mit commented Oct 26, 2023

No description provided.

Copy link
Owner

@lrhn lrhn left a comment

Choose a reason for hiding this comment

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

Very good! Nits on phrasings.

## From `dart:io`

The `dart:io` core library is a special custom library only available on Dart
native platforms (and for example, not available on Dart web platforms). We
Copy link
Owner

Choose a reason for hiding this comment

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

I'd make a newline before "We", to make reading the source easier. (Semantic line-breaks FTW!)

```dart
import 'package:platform/platform.dart'; // version 4.x

bool onAndroid = Platform.current.isAndroid;
Copy link
Owner

@lrhn lrhn Oct 27, 2023

Choose a reason for hiding this comment

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

Should actually be

import 'package:platform/host.dart';

bool onAndroid = HostPlatform.current.isAndroid;

The platform.dart library is not cross-platform capable, it's there for backwards compatibility with v3, where platform.dart allowed access to native-only features.

So, the question is whether this is a minimal migration guide or a full migration guide.

The migration shown here is the minimal needed to preserve the current behavior, including the dependency on dart:io.

The one I wrote above is the ultimate migration, to cross-platform compatible code.

Which one do we want? Both?

Copy link
Author

Choose a reason for hiding this comment

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

I would like it if we could do the full migration to HostPlatform, but whether that is realistic I think depends on whether dart fix can automate the migration. I'm going to share this page with the analyzer team to get their input.

@mit-mit
Copy link
Author

mit-mit commented Nov 6, 2023

Can we merge @lrhn (this is your repo, so I can't do it) ?

@lrhn lrhn merged commit 28faf89 into lrhn:4.0-os-detect Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants