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

Make Cancellable Send+Sync and don't generate trait for it #102

Merged
merged 1 commit into from Apr 9, 2018

Conversation

Projects
None yet
4 participants
@sdroege
Member

sdroege commented Apr 8, 2018

It can't be subclassed.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 8, 2018

Member

👍 without version.txt change

Member

EPashkin commented Apr 8, 2018

👍 without version.txt change

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 8, 2018

Member

Without this, Cancellable is basically useless as you would use it to cancel something that blocks your current thread usually :)

Member

sdroege commented Apr 8, 2018

Without this, Cancellable is basically useless as you would use it to cancel something that blocks your current thread usually :)

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 9, 2018

Member

Should be good to go then

Member

sdroege commented Apr 9, 2018

Should be good to go then

@tmiasko

This comment has been minimized.

Show comment
Hide comment
@tmiasko

tmiasko Apr 9, 2018

Contributor

If you don't need reset I would drop it from bindings.

Contributor

tmiasko commented Apr 9, 2018

If you don't need reset I would drop it from bindings.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 9, 2018

Member

If you don't need reset I would drop it from bindings

Why?

Member

sdroege commented Apr 9, 2018

If you don't need reset I would drop it from bindings

Why?

@tmiasko

This comment has been minimized.

Show comment
Hide comment
@tmiasko

tmiasko Apr 9, 2018

Contributor

I should have included this earlier:

If cancellable is currently in use by any cancellable operation then the behavior of this function is undefined.

Contributor

tmiasko commented Apr 9, 2018

I should have included this earlier:

If cancellable is currently in use by any cancellable operation then the behavior of this function is undefined.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 9, 2018

Member

Ugly indeed :) I send another PR for that later

Member

sdroege commented Apr 9, 2018

Ugly indeed :) I send another PR for that later

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 9, 2018

Member

Code looks good to me. Should I wait for the other PR before merging this one?

Member

GuillaumeGomez commented Apr 9, 2018

Code looks good to me. Should I wait for the other PR before merging this one?

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 9, 2018

Member

They're independent things

Member

sdroege commented Apr 9, 2018

They're independent things

sdroege added a commit to sdroege/gio-rs that referenced this pull request Apr 9, 2018

Remove Cancellable::reset()
It can easily cause undefined behaviour, see
gtk-rs#102 (comment)
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 9, 2018

Member

Ok! Thanks for the PR! :)

Member

GuillaumeGomez commented Apr 9, 2018

Ok! Thanks for the PR! :)

@GuillaumeGomez GuillaumeGomez merged commit 6eaeb60 into gtk-rs:master Apr 9, 2018

2 checks passed

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

sdroege added a commit to sdroege/gio-rs that referenced this pull request Apr 17, 2018

Remove Cancellable::reset()
It can easily cause undefined behaviour, see
gtk-rs#102 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment