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

Regen #209

Merged
merged 2 commits into from Aug 1, 2017

Conversation

Projects
None yet
3 participants
@EPashkin
Member

EPashkin commented Jul 31, 2017

Apply gtk-rs/gir#419, gtk-rs/gir#418 and appveyour version fix

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin
Member

EPashkin commented Jul 31, 2017

@sdroege

👍 but you might want to add the GCheckSum things :)

Show outdated Hide outdated src/auto/functions.rs
@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 31, 2017

Member

Looks good :) I also thought of GCheckSum but that can also be added later when needed

Member

sdroege commented Jul 31, 2017

Looks good :) I also thought of GCheckSum but that can also be added later when needed

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 31, 2017

Member

I missed GCheckSum, but it g_checksum_get_digest has wrong gir definition, it can be used without this function?

Member

EPashkin commented Jul 31, 2017

I missed GCheckSum, but it g_checksum_get_digest has wrong gir definition, it can be used without this function?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Jul 31, 2017

Member

As compute_checksum_for_XX return string, seems GCheckSum still usable without bad function, so I add it too.

Member

EPashkin commented Jul 31, 2017

As compute_checksum_for_XX return string, seems GCheckSum still usable without bad function, so I add it too.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Jul 31, 2017

Member

Looks good but get_digest would be useful... I guess that needs a manually defined extern "C" function in glib-rs as a workaround for now :(

Member

sdroege commented Jul 31, 2017

Looks good but get_digest would be useful... I guess that needs a manually defined extern "C" function in glib-rs as a workaround for now :(

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 1, 2017

Member

Good to go in any case, unless you want to add that manual get_digest implementation :)

Member

sdroege commented Aug 1, 2017

Good to go in any case, unless you want to add that manual get_digest implementation :)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 1, 2017

Member

I add it at evening with tests for Checksum

Member

EPashkin commented Aug 1, 2017

I add it at evening with tests for Checksum

pub fn get_string(self) -> Option<String> {
unsafe {
from_glib_none(ffi::g_checksum_get_string(

This comment has been minimized.

@EPashkin

EPashkin Aug 1, 2017

Member

Made this function manual to consume Checksum and prevent next updates

@EPashkin

EPashkin Aug 1, 2017

Member

Made this function manual to consume Checksum and prevent next updates

This comment has been minimized.

@sdroege

sdroege Aug 1, 2017

Member

Makes sense

@sdroege

sdroege Aug 1, 2017

Member

Makes sense

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 1, 2017

Member

@GuillaumeGomez, @sdroege Not sure that it right way implement get_digest.
Also no way to get type from Checksum and calculate length, so I used 4096 as initial buffer size.

Member

EPashkin commented Aug 1, 2017

@GuillaumeGomez, @sdroege Not sure that it right way implement get_digest.
Also no way to get type from Checksum and calculate length, so I used 4096 as initial buffer size.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 1, 2017

Member

Maybe we need add cargo test for travis too.

Member

EPashkin commented Aug 1, 2017

Maybe we need add cargo test for travis too.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
Member

EPashkin commented Aug 1, 2017

Show outdated Hide outdated src/checksum.rs
Show outdated Hide outdated src/checksum.rs
Show outdated Hide outdated src/checksum.rs
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 1, 2017

Member

@sdroege, Fixed, Thanks.

Member

EPashkin commented Aug 1, 2017

@sdroege, Fixed, Thanks.

ChecksumType::Sha1 => ffi::G_CHECKSUM_SHA1,
ChecksumType::Sha256 => ffi::G_CHECKSUM_SHA256,
#[cfg(feature = "v2_36")]
ChecksumType::Sha512 => ffi::G_CHECKSUM_SHA512,

This comment has been minimized.

@sdroege

sdroege Aug 1, 2017

Member

There's also G_CHECKSUM_SHA384 since 2.51. I think you should update your .gir files at some point in the near future :)

@sdroege

sdroege Aug 1, 2017

Member

There's also G_CHECKSUM_SHA384 since 2.51. I think you should update your .gir files at some point in the near future :)

This comment has been minimized.

@EPashkin

EPashkin Aug 1, 2017

Member

Yes, we will update it.

@EPashkin

EPashkin Aug 1, 2017

Member

Yes, we will update it.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 1, 2017

Member

Changed buffer size

Member

EPashkin commented Aug 1, 2017

Changed buffer size

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 1, 2017

Member

👍 if you also add a comment about that it has to be updated whenever a bigger hash is supported

Member

sdroege commented Aug 1, 2017

👍 if you also add a comment about that it has to be updated whenever a bigger hash is supported

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 1, 2017

Member

That will do?

Member

EPashkin commented Aug 1, 2017

That will do?

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 1, 2017

Member

Sure

Member

sdroege commented Aug 1, 2017

Sure

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 1, 2017

Member

Well, I didn't have any issue at the start so if it's ok for @sdroege too then let's merge! :D

Member

GuillaumeGomez commented Aug 1, 2017

Well, I didn't have any issue at the start so if it's ok for @sdroege too then let's merge! :D

@GuillaumeGomez GuillaumeGomez merged commit 616033c into gtk-rs:master Aug 1, 2017

2 checks passed

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

@EPashkin EPashkin deleted the EPashkin:Regen branch Aug 1, 2017

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