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 futures variants for all async functions. #108

Merged
merged 6 commits into from Apr 20, 2018

Conversation

Projects
None yet
4 participants
@sdroege
Member

sdroege commented Apr 18, 2018

This depends on gtk-rs/gir#582 and gtk-rs/glib#314

For an example usage see the memory input stream unit test, I'll also add a more involved example to the examples repository once everything is merged

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 19, 2018

Member

@GuillaumeGomez Please restart CI here

Member

EPashkin commented Apr 19, 2018

@GuillaumeGomez Please restart CI here

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 19, 2018

Member

This currently uses my own branch of glib, switching back :)

Member

sdroege commented Apr 19, 2018

This currently uses my own branch of glib, switching back :)

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 19, 2018

Member

I'm also currently working on rebasing this and making it work with @tmiasko's changes (except for the rename of all the files)

Member

sdroege commented Apr 19, 2018

I'm also currently working on rebasing this and making it work with @tmiasko's changes (except for the rename of all the files)

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 19, 2018

Member

I'm currently cleaning up this PR (please get the other ones merged) and making things work for non-method async operations.

Member

sdroege commented Apr 19, 2018

I'm currently cleaning up this PR (please get the other ones merged) and making things work for non-method async operations.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 19, 2018

Member

new cleaned up version of everything, this handles global functions too now

Member

sdroege commented Apr 19, 2018

new cleaned up version of everything, this handles global functions too now

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 19, 2018

Member

This should be good to go now

Member

sdroege commented Apr 19, 2018

This should be good to go now

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 19, 2018

Member

Ah I did not know that. I'll remove the docs here then

Member

sdroege commented Apr 19, 2018

Ah I did not know that. I'll remove the docs here then

Show outdated Hide outdated src/lib.rs
Show outdated Hide outdated src/lib.rs

@sdroege sdroege referenced this pull request Apr 20, 2018

Merged

Add gio_futures example #166

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

Rebased to remove all the other PRs that are in master now. This should be good to go.

And as a next step we can regenerate all crate with latest GIR (+ file renames).

Member

sdroege commented Apr 20, 2018

Rebased to remove all the other PRs that are in master now. This should be good to go.

And as a next step we can regenerate all crate with latest GIR (+ file renames).

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

This should be good to go now, so that afterwards we can regenerate everything properly. @GuillaumeGomez?

Member

sdroege commented Apr 20, 2018

This should be good to go now, so that afterwards we can regenerate everything properly. @GuillaumeGomez?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 20, 2018

Member

It contains new generated code but no submodule updates, so travis fails.
I restarted appveyor, it IMHO be fixed now.

Member

EPashkin commented Apr 20, 2018

It contains new generated code but no submodule updates, so travis fails.
I restarted appveyor, it IMHO be fixed now.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

It contains new generated code but no submodule updates, so travis fails.

I see, but we can't use latest GIR because that already contains all the file renames :) My suggestion would be to get this here in first, and then we fix up all these things by regenerating everything one by one

Member

sdroege commented Apr 20, 2018

It contains new generated code but no submodule updates, so travis fails.

I see, but we can't use latest GIR because that already contains all the file renames :) My suggestion would be to get this here in first, and then we fix up all these things by regenerating everything one by one

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 20, 2018

Member

Don't you already do regen with latest gir?
I have only versions.txt change on local update.

Member

EPashkin commented Apr 20, 2018

Don't you already do regen with latest gir?
I have only versions.txt change on local update.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 20, 2018

Member

Hm, there both old and new files here.

Member

EPashkin commented Apr 20, 2018

Hm, there both old and new files here.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

No, because the latest GIR would pull in the file renames and then we have a huge diff with not only the futures addition but also the renames and it would be very hard to follow from the GIT history what exactly happened/changed.

Member

sdroege commented Apr 20, 2018

No, because the latest GIR would pull in the file renames and then we have a huge diff with not only the futures addition but also the renames and it would be very hard to follow from the GIT history what exactly happened/changed.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

Hrm indeed. We have everything at once here already, I must've updated the GIR version at some point.

Ok so how to proceed? First without renames, then another with renames? Or all at once?

Member

sdroege commented Apr 20, 2018

Hrm indeed. We have everything at once here already, I must've updated the GIR version at some point.

Ok so how to proceed? First without renames, then another with renames? Or all at once?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 20, 2018

Member

IMHO better remove old version in same commit, so git show that file moved.

Member

EPashkin commented Apr 20, 2018

IMHO better remove old version in same commit, so git show that file moved.

@tmiasko

This comment has been minimized.

Show comment
Hide comment
@tmiasko

tmiasko Apr 20, 2018

Contributor

I would fix the commit 8f48312, then in additional commit rm -r auto, update submodules and regenerate with renames so that CI pass.

Contributor

tmiasko commented Apr 20, 2018

I would fix the commit 8f48312, then in additional commit rm -r auto, update submodules and regenerate with renames so that CI pass.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

Ok let's do that then

Member

sdroege commented Apr 20, 2018

Ok let's do that then

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

This should be good now. One commit for the futures addition, another for the renames and updating to latest GIR.

Member

sdroege commented Apr 20, 2018

This should be good now. One commit for the futures addition, another for the renames and updating to latest GIR.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 20, 2018

Member

Yes, only one possible change is run tests for futures too.

Member

EPashkin commented Apr 20, 2018

Yes, only one possible change is run tests for futures too.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 20, 2018

Member

Strange, that appveyor don't run tests at all, but this IMHO better fix in separate PR.

Member

EPashkin commented Apr 20, 2018

Strange, that appveyor don't run tests at all, but this IMHO better fix in separate PR.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

Yes let's not enable more tests until all tests are actually succeeding on CI again

Member

sdroege commented Apr 20, 2018

Yes let's not enable more tests until all tests are actually succeeding on CI again

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

travis is green, nice

Member

sdroege commented Apr 20, 2018

travis is green, nice

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 20, 2018

Member

Apveyor good too.

Member

EPashkin commented Apr 20, 2018

Apveyor good too.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 20, 2018

Member

Great :) let's get this in then?

Member

sdroege commented Apr 20, 2018

Great :) let's get this in then?

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 20, 2018

Member

Good for me as well. Thanks!

Member

GuillaumeGomez commented Apr 20, 2018

Good for me as well. Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 8aef161 into gtk-rs:master Apr 20, 2018

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Apr 21, 2018

Member

Thanks! I send PRs for everything else this afternoon

Member

sdroege commented Apr 21, 2018

Thanks! I send PRs for everything else this afternoon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment