Skip to content

Rename Border.uniform() -> Border.fromSide() #30792

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
tvolkert merged 2 commits into from
Apr 17, 2019
Merged

Rename Border.uniform() -> Border.fromSide() #30792

tvolkert merged 2 commits into from
Apr 17, 2019

Conversation

@tvolkert
Copy link
Contributor

@tvolkert tvolkert commented Apr 9, 2019

No description provided.

@tvolkert tvolkert requested a review from Hixie April 9, 2019 16:32
@tvolkert tvolkert mentioned this pull request Apr 9, 2019
Merged
9 tasks
jonahwilliams
jonahwilliams approved these changes Apr 9, 2019
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams 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
Copy link
Contributor

dnfield commented Apr 9, 2019

When I read fromSide I end up wondering which side. IMO uniform is clearer.

@tvolkert
Copy link
Contributor Author

tvolkert commented Apr 9, 2019

fwiw, I can see both sides of it (truly no pun intended) and don't feel strongly either way. I like both fromSide() and uniform().

@dnfield
Copy link
Contributor

dnfield commented Apr 9, 2019

Maybe fromUniformSide? Although that's getting back closer to where this all started. What do you think @jonahwilliams ?

@goderbauer goderbauer added the framework flutter/packages/flutter repository. See also f: labels. label Apr 9, 2019
@Hixie
Copy link
Contributor

Hixie commented Apr 9, 2019

fromBorderSide maybe?

The problem with uniform is that that means the same as all and it's not clear why you'd use one or the other from the name.

@tvolkert
Copy link
Contributor Author

tvolkert commented Apr 9, 2019

fromBorderSide is definitely unambiguous. +1 from me.

@dnfield
Copy link
Contributor

dnfield commented Apr 9, 2019

My understanding is that const is the disambiguator here - you use this one when you want a const version and are willing to explicitly provide a borderside rather than the individual properties that .all expects. But fromBorderSide is a bit clearer at least.

@dnfield
Copy link
Contributor

dnfield commented Apr 10, 2019

LGTM

@tvolkert tvolkert merged commit a4ab032 into flutter:master Apr 17, 2019
@tvolkert tvolkert deleted the fromSide branch April 17, 2019 17:58
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Hixie Hixie Awaiting requested review from Hixie

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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.

6 participants

@tvolkert @dnfield @Hixie @jonahwilliams @goderbauer @googlebot