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

Adding protections for invalid flex parameters #1691

Merged
merged 16 commits into from
Apr 6, 2021

Conversation

arthmis
Copy link
Collaborator

@arthmis arthmis commented Apr 2, 2021

Ok, not sure if there are better ways to clamp a value above a certain threshold. Also, is it fine to do the tests like that? I had to do it that way otherwise the test fails in release and passes debug.

@arthmis arthmis added the S-needs-review waits for review label Apr 2, 2021
@arthmis arthmis changed the title Flex params safe Adding protections for invalid flex parameters Apr 2, 2021
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.

Thanks! A couple notes inline :)

druid/src/widget/flex.rs Outdated Show resolved Hide resolved
druid/src/widget/flex.rs Show resolved Hide resolved
druid/src/widget/flex.rs Outdated Show resolved Hide resolved
druid/src/widget/flex.rs Outdated Show resolved Hide resolved
@arthmis
Copy link
Collaborator Author

arthmis commented Apr 3, 2021

I opted to keep add_child within with_child instead of relying on the add_flex_child function checking for flex values that are 0.0 or less.

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.

Looks good! A few little formatting nits, but happy to see this merged. :)

druid/src/widget/flex.rs Outdated Show resolved Hide resolved
druid/src/widget/flex.rs Outdated Show resolved Hide resolved
druid/src/widget/flex.rs Outdated Show resolved Hide resolved
@SecondFlight SecondFlight added S-waiting-on-author waits for changes from the submitter and removed S-needs-review waits for review labels Apr 5, 2021
@cmyr
Copy link
Member

cmyr commented Apr 5, 2021

What's the conclusion here?

@SecondFlight SecondFlight removed the S-waiting-on-author waits for changes from the submitter label Apr 5, 2021
@arthmis
Copy link
Collaborator Author

arthmis commented Apr 5, 2021

In regards to what? Are there any other issues? The only one that came up was the tracing events need a subscriber created before the app is launched but that is a separate issue.

oh oops, I'm going to take out the tracing instrument for the add flex child function. I forgot. Other than that I think everything is fine.

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.

Thanks!

@cmyr cmyr merged commit 39238ad into linebender:master Apr 6, 2021
@arthmis arthmis mentioned this pull request Apr 7, 2021
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

3 participants