8000 Prefer .of to .from by dnfield · Pull Request #94772 · flutter/flutter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Prefer .of to .from #94772

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 7, 2021
Merged

Prefer .of to .from #94772

merged 1 commit into from
Dec 7, 2021

Conversation

dnfield
Copy link
Contributor
@dnfield dnfield commented Dec 7, 2021 •
edited
Loading

.from uses a typecast.

.of does not.

We should have a lint for this - see dart-lang/sdk#58359

There's not really a good way to test this right now. What we really want is a lint.

For historical context: my understanding is that from is more of a Dart 1 thing that was the original go-to and of was added later to make better use of types.

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: animation Animation APIs f: cupertino flutter/packages/flutter/cupertino repository f: focus Focus traversal, gaining or losing focus f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels Dec 7, 2021
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@dnfield dnfield requested a review from goderbauer December 7, 2021 01:13
@Hixie
Copy link
Contributor
Hixie commented Dec 7, 2021

Why not just .toList()?

@Hixie
Copy link
Contributor
Hixie commented Dec 7, 2021

test-exempt: preparing codebase for future lint (dart-lang/sdk#58359)

@dnfield
Copy link
Contributor Author
dnfield commented Dec 7, 2021

These aren't always lists.

@Hixie
Copy link
Contributor
Hixie commented Dec 7, 2021 •
edited
Loading

the List.of() cases are always Iterables though right? And can always be replaced by toList?

@dnfield
Copy link
Contributor Author
dnfield commented Dec 7, 2021

Presumably yes. But I got here from looking at maps

@goderbauer
Copy link
Member

The implementation of toList is to forward to List.of: https://master-api.flutter.dev/flutter/dart-core/Iterable/toList.html

@Hixie
Copy link
Contributor
Hixie commented Dec 7, 2021

fair enough.

FWIW in general i feel toList is more idiomatic (and shorter) but I don't have a strong opinion here.

goderbauer
goderbauer approved these changes Dec 7, 2021
View reviewed changes
Copy link
Member
@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@dnfield dnfield merged commit e4a1d3e into flutter:master Dec 7, 2021
@dnfield dnfield deleted the from_of branch December 7, 2021 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goderbauer goderbauer goderbauer approved these changes

Assignees
No one assigned
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: animation Animation APIs f: cupertino flutter/packages/flutter/cupertino repository f: focus Focus traversal, gaining or losing focus f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dnfield @Hixie @goderbauer
0