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 GDate & GDateTime (and related) bindings #208

Merged
merged 3 commits into from Aug 2, 2017

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Jul 30, 2017

No description provided.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 30, 2017

Member

src/date.rs was autogenerated with gir by cheating: I edited the .gir file to add a copy function, and then implemented that one correctly afterwards. I don't think we can teach gir to handle this correctly, as without any copy/free functions we don't know if this is with reference counting or copying.

Member

sdroege commented Jul 30, 2017

src/date.rs was autogenerated with gir by cheating: I edited the .gir file to add a copy function, and then implemented that one correctly afterwards. I don't think we can teach gir to handle this correctly, as without any copy/free functions we don't know if this is with reference counting or copying.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 30, 2017

Member

I'm a bit worry about this: if you edited the .gir file, it means only you can generate this for the moment, right?

Member

GuillaumeGomez commented Jul 30, 2017

I'm a bit worry about this: if you edited the .gir file, it means only you can generate this for the moment, right?

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 30, 2017

Member

That's why it is in src/date.rs and not src/auto/date.rs. The problem here is that we'll never be able to autogenerate this properly unless GLib is adding a copy function.

Member

sdroege commented Jul 30, 2017

That's why it is in src/date.rs and not src/auto/date.rs. The problem here is that we'll never be able to autogenerate this properly unless GLib is adding a copy function.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Jul 30, 2017

Member

Ah I see, my bad. (it's awesome!)

Member

GuillaumeGomez commented Jul 30, 2017

Ah I see, my bad. (it's awesome!)

Show outdated Hide outdated src/date.rs Outdated
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 31, 2017

Member

Code looks good, but I prefer that it wait for #209 and then rebased and regened with rights hashes

Member

EPashkin commented Jul 31, 2017

Code looks good, but I prefer that it wait for #209 and then rebased and regened with rights hashes

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 31, 2017

Member

This probably applies fine once #209 is merged, or it's a trivial rebase. Will check then

Member

sdroege commented Jul 31, 2017

This probably applies fine once #209 is merged, or it's a trivial rebase. Will check then

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 1, 2017

Member

This should be good to go now. Rebased

Member

sdroege commented Aug 1, 2017

This should be good to go now. Rebased

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 1, 2017

Member

Later on, it'd be nice to have conversions with rust std equivalents. But that's completely ok to do it in another PR. Just waiting for @EPashkin since he said he wanted to check again. :)

Member

GuillaumeGomez commented Aug 1, 2017

Later on, it'd be nice to have conversions with rust std equivalents. But that's completely ok to do it in another PR. Just waiting for @EPashkin since he said he wanted to check again. :)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 2, 2017

Member

@sdroege Thanks.
Only nit is that generate array in Gir.toml need sort.

Member

EPashkin commented Aug 2, 2017

@sdroege Thanks.
Only nit is that generate array in Gir.toml need sort.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 2, 2017

Member

Done :)

Member

sdroege commented Aug 2, 2017

Done :)

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 2, 2017

Member

Thanks!

Member

GuillaumeGomez commented Aug 2, 2017

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 8eacb19 into gtk-rs:master Aug 2, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment