Skip to content
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

Take root widget directly in WindowDesc::new #1559

Merged
merged 3 commits into from
Feb 3, 2021

Conversation

lassipulkkinen
Copy link
Contributor

@lassipulkkinen lassipulkkinen commented Jan 31, 2021

WindowDesc::new currently accepts the root widget as a closure that returns W: Widget<_>, which made sense before the changes in #519. This PR changes that to take W directly instead, as using a closure serves no purpose anymore.

EDIT: Sorry for the force push mess; I forgot to cargo fmt. Also, reverted AUTHORS change on second thought, as this is probably not eligible for copyright.

@cmyr
Copy link
Member

cmyr commented Feb 3, 2021

/bloat

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

ha funny, I'd forgotten why this API was like this, but I'm pretty sure you're right.

I do think that this is a simplification, and I'm happy to merge, once we figure out what's up with CI.

@github-actions
Copy link

github-actions bot commented Feb 3, 2021

🗜 Bloat check ⚖️

Comparing ef17edd against 4c865b9

target old size new size difference
target/release/examples/calc 1.42 MB 1.42 MB 56 Bytes (0.00%)
target/release/examples/scroll_colors 1.41 MB 1.41 MB 320 Bytes (0.02%)
target/release/examples/multiwin 1.42 MB 1.42 MB -16 Bytes (-0.00%)
target/release/examples/flex 1.85 MB 1.85 MB -48 Bytes (-0.00%)
target/release/examples/styled_text 1.64 MB 1.64 MB -56 Bytes (-0.00%)
target/release/examples/custom_widget 1.33 MB 1.33 MB 96 Bytes (0.01%)

@cmyr cmyr merged commit f391759 into linebender:master Feb 3, 2021
@lassipulkkinen lassipulkkinen deleted the windowdesc-new-no-closure branch February 3, 2021 20:31
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.

None yet

2 participants