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

Remove xx_sys to xx_ffi renaming #470

Merged
merged 1 commit into from Mar 10, 2019

Conversation

Projects
None yet
3 participants
@EPashkin
Copy link
Member

commented Mar 5, 2019

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

👍

@EPashkin EPashkin force-pushed the EPashkin:ffi_to_sys branch 2 times, most recently from 86924a5 to 3d5f152 Mar 5, 2019

@@ -3,43 +3,43 @@
// DO NOT EDIT

use ChecksumType;
use ffi;
use sys;

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 6, 2019

Member

If we're changing this anyway, shouldn't we directly name this glib_sys instead? That way there's no renaming necessary at all even for the "local" FFI bindings and it would be more consistent.

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 6, 2019

Author Member

Currently I think that all concrete classes better be almost same as generated code (use sys for current FFI-crate).
In glib has 2 main parts: currently glib-based object has sys+gobject_sys,
only gobject-based classes has glib_sys+gobject_sys to minimize confusion.
If we want use xx_sys instead sys in generated code too, it maybe need loot changes in gir.

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 6, 2019

Member

I'd prefer to go without sys and rename it all to xxx_sys.

As with Rust 2018 the extern crate are not required anymore, it would be really nice if we wouldn't have to do renames. It simplifies things a lot :)

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 6, 2019

Author Member

Ok I try change this too.

This comment has been minimized.

Copy link
@sdroege

sdroege Mar 6, 2019

Member

Great, thanks :)

@EPashkin EPashkin force-pushed the EPashkin:ffi_to_sys branch from 3d5f152 to d833c98 Mar 6, 2019

@EPashkin

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2019

Now always glib_sys/gobject_sys

@sdroege

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Now always glib_sys/gobject_sys

Awesome. All good for me then :)

@sdroege

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

Why is this WIP? Good to go in any case with the latest version of the gir PR.

@EPashkin

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2019

All regen PR is WIP as it don't have right gir, I just forgot addd WIP to gtk

@sdroege

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

All regen PR is WIP as it don't have right gir, I just forgot addd WIP to gtk

gir PR is merged now, so let's get this updated and merged? :)

@EPashkin

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2019

I will update all PRs today after gtk-rs/gir#740

@EPashkin EPashkin force-pushed the EPashkin:ffi_to_sys branch from d833c98 to 2d4d680 Mar 10, 2019

@EPashkin EPashkin changed the title WIP: Remove xx_sys to xx_ffi renaming Remove xx_sys to xx_ffi renaming Mar 10, 2019

@EPashkin

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2019

Updated, ready for final check

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

Still 👍

@EPashkin EPashkin force-pushed the EPashkin:ffi_to_sys branch from 2d4d680 to ffcb4b3 Mar 10, 2019

@EPashkin

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2019

Fixed tests

@sdroege

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

Looks good to me

@EPashkin

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2019

CI "passed"

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

commented Mar 10, 2019

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit a3d0ab7 into gtk-rs:master Mar 10, 2019

1 of 2 checks passed

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

@EPashkin EPashkin deleted the EPashkin:ffi_to_sys branch Mar 10, 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.