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

subclassing: move parent invocation fnct in Ext trait #450

Merged
merged 1 commit into from Feb 13, 2019

Conversation

Projects
None yet
2 participants
@fengalin
Copy link
Contributor

fengalin commented Feb 13, 2019

This reduces the risk for the user to override them, or even worse, let the user believe that it has an actual effect to override them.

See https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/merge_requests/223#note_112472

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 13, 2019

This reduces the risk for the user to override them.

Or worse, the user believing that it has an actual effect to override them :)

@fengalin

This comment has been minimized.

Copy link
Contributor Author

fengalin commented Feb 13, 2019

Or worse, the user believing that it has an actual effect to override them :)

Indeed :) Do you want me to amend the commit message?

pub trait ObjectImplExt {
/// Chain up to the parent class' implementation of `glib::Object::constructed()`.
///
/// Do not override this, it has no effect.

This comment has been minimized.

@sdroege

sdroege Feb 13, 2019

Member

Well, it can't be overridden anymore so this part of the docs can go away :)

This comment has been minimized.

@fengalin

fengalin Feb 13, 2019

Author Contributor

Done! :)

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 13, 2019

Looks good to me apart from that line in the docs, thanks!

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 13, 2019

Indeed :) Do you want me to amend the commit message?

No that's fine

@fengalin

This comment has been minimized.

Copy link
Contributor Author

fengalin commented Feb 13, 2019

Note that AppVeyor tests are marked as "succeeded" when the 64bits version is still in progress.

subclassing: move parent invocation fnct in Ext trait
This reduces the risk for the user to override them, or even worse,
let the user believe that it has an actual effect to override them.

See https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/merge_requests/223#note_112472

@fengalin fengalin force-pushed the fengalin:subclass_parent_ext branch from cc4606a to 2f4e876 Feb 13, 2019

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Feb 13, 2019

Thanks, will merge once CI is happy again

@sdroege sdroege merged commit f33d743 into gtk-rs:master Feb 13, 2019

2 checks passed

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

@fengalin fengalin deleted the fengalin:subclass_parent_ext branch Feb 13, 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.