Skip to content
This repository was archived by the owner on Oct 23, 2024. It is now read-only.

Allowing instances class to be extensible.#31

Merged
naemono merged 2 commits into
masterfrom
allow_instances_extensible
Jan 5, 2016
Merged

Allowing instances class to be extensible.#31
naemono merged 2 commits into
masterfrom
allow_instances_extensible

Conversation

@naemono

@naemono naemono commented Jan 4, 2016

Copy link
Copy Markdown
Contributor

Version bump
Changelog update.

Version bump
Changelog update.
@naemono naemono self-assigned this Jan 4, 2016
Comment thread objectrocket/instances/__init__.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep the BaseOperationsLayer at the front of the bases declaration.

Per python's new-style class MRO, the left-most and lowest occurring class in a hierarchy will have its method selected first at resolution time. So, to ensure that the BaseOperationsLayer.__init__ is always called first, we should keep it as the first item in this bases declaration. Under certain circumstances, the __init__ could be completely skipped — say, if someone decided to add an __init__ to the Extensible mixin, or added a new base to this class at the front of the bases declaration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent point. Updates made @thedodd . Will merge once checks complete.

@thedodd

thedodd commented Jan 4, 2016

Copy link
Copy Markdown
Contributor

+1 after inline comment.

naemono added a commit that referenced this pull request Jan 5, 2016
Allowing instances class to be extensible.
@naemono
naemono merged commit 51a1482 into master Jan 5, 2016
@naemono
naemono deleted the allow_instances_extensible branch January 5, 2016 14:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants