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

Update ResponseType #726

Merged
merged 1 commit into from Oct 12, 2018

Conversation

Projects
None yet
3 participants
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Oct 11, 2018

Fixes #725.

cc @sdroege @EPashkin

-9 => ResponseType::No,
-10 => ResponseType::Apply,
-11 => ResponseType::Help,
value if value >= 0 => ResponseType::Other(value as u16),

This comment has been minimized.

@EPashkin

EPashkin Oct 12, 2018

Member

Maybe with && value <= u16:MAX ?

This comment has been minimized.

@sdroege
@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Oct 12, 2018

Looks good except for the one comment @EPashkin wrote

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:response-type branch from 0e34e0f to 8453962 Oct 12, 2018

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Oct 12, 2018

Updated.

Help,
Other(u16),
#[doc(hidden)]
Unknown(i32),

This comment has been minimized.

@sdroege

sdroege Oct 12, 2018

Member

Why did it lose its two underscores in the beginning?

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Oct 12, 2018

Author Member

Want me to put them back? At first I just replaced it with Other and then put it back.

This comment has been minimized.

@sdroege

sdroege Oct 12, 2018

Member

I'd prefer that with underscores, yes

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:response-type branch from 8453962 to 39e9e8e Oct 12, 2018

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Oct 12, 2018

Updated Unknown.

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Oct 12, 2018

👍

@GuillaumeGomez GuillaumeGomez merged commit 8a4700d into gtk-rs:master Oct 12, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:response-type branch Oct 12, 2018

vhdirk pushed a commit to vhdirk/gtk-rs that referenced this pull request Jan 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.