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

Add a special type for add_tick_callback's return value #792

Merged
merged 2 commits into from Mar 3, 2019

Conversation

Projects
None yet
4 participants
@sfanxiang
Copy link
Contributor

commented Mar 1, 2019

For safety reasons, the value consumed by remove_tick_callback needs to be created by add_tick_callback. FromGlib and ToGlib are not implemented for the same reason.

First contribution, so I may have missed things.

Fixes #761, cc @EPashkin, @sdroege.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

Nope, seems good globally after a quick look. I'll go back to it later on. cc @EPashkin @sdroege

Show resolved Hide resolved src/widget.rs Outdated
@sdroege

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

Looks good otherwise

})
}

fn remove_tick_callback(&self, id: TickCallbackId) {

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 1, 2019

Member

Not better add this function to TickCallbackId ?

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 1, 2019

Member

And please add #[allow(clippy::needless_pass_by_value)] if clippy nags.

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 1, 2019

Member

@GuillaumeGomez, @sdroege You agree to moving remove function to type itself?

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 1, 2019

Member

Works for me

This comment has been minimized.

Copy link
@sfanxiang

sfanxiang Mar 2, 2019

Author Contributor

Done, but TickCallbackId no longer implements Debug, Eq, PartialEq, and Send because WeakRef doesn't.

@sfanxiang sfanxiang force-pushed the sfanxiang:tick-callback branch 3 times, most recently from 0e3209c to ff52a30 Mar 2, 2019

@EPashkin EPashkin referenced this pull request Mar 2, 2019

Merged

Fix doctest #52

Show resolved Hide resolved src/widget.rs Outdated

sfanxiang added some commits Mar 1, 2019

Add a special type for add_tick_callback's return value
For safety reasons, the value consumed by remove_tick_callback needs to
be created by add_tick_callback. FromGlib and ToGlib are not implemented
for the same reason.

@sfanxiang sfanxiang force-pushed the sfanxiang:tick-callback branch from ff52a30 to a0036e4 Mar 2, 2019

@EPashkin

This comment has been minimized.

Copy link
Member

commented Mar 2, 2019

@sfanxiang Thanks.
👍 after travis passed.

@sfanxiang

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2019

@EPashkin

after travis passed.

Travis passed, but idk why appveyor BITS=32 failed.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Mar 3, 2019

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 9ed2df3 into gtk-rs:master Mar 3, 2019

1 of 2 checks passed

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

@sfanxiang sfanxiang deleted the sfanxiang:tick-callback branch Mar 3, 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.