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

Replace fixed class method calls with self(.class) in Unit class #241

Merged
merged 2 commits into from
Nov 23, 2021

Conversation

kreintjes
Copy link
Contributor

Replaces every class method call RubyUnits::Unit.method with either method
(in class methods) or self.class.method (in instance methods).

This allows us to override a class method in a sub class and make sure that
every call to that method from it's instances use the new definition.

E.g. we can have a sub class which extends from RubyUnits::Unit and override
eliminate_terms which is then used when dividing or multiplicating instances
of this sub class.

Replaces every class method call `RubyUnits::Unit.method` with either `method`
(in class methods) or `self.class.method` (in instance methods).

This allows us to override a class method in a sub class and make sure that
every call to that method from it's instances use the new definition.

E.g. we can have a sub class which extends from RubyUnits::Unit and override
eliminate_terms which is then used when dividing or multiplicating instances
of this sub class.
@olbrich olbrich self-assigned this Nov 2, 2021
@olbrich olbrich added feature request Minor add functionality in a backwards-compatible manner labels Nov 2, 2021
Copy link
Owner

@olbrich olbrich left a comment

Choose a reason for hiding this comment

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

👍 Looks good. Thanks for the contribution!

@olbrich
Copy link
Owner

olbrich commented Nov 21, 2021

@kreintjes If you update this with master, I'll merge it in.

@kreintjes
Copy link
Contributor Author

Thanks for the review!

@kreintjes If you update this with master, I'll merge it in.

Do you prefer a rebase or merge commit? If a merge commit is fine, then I'll press this button:

image

Otherwise it will have to wait a bit longer.

@olbrich
Copy link
Owner

olbrich commented Nov 23, 2021

@kreintjes a merge commit is fine, I'm going to squash the branch anyway when I merge it. Go ahead and press the button.

@kreintjes
Copy link
Contributor Author

@kreintjes a merge commit is fine, I'm going to squash the branch anyway when I merge it. Go ahead and press the button.

I was now in the position to do either (but actually kind of forgot about this :p), but pressing the button is much easier anyway, so done!

@olbrich olbrich merged commit 8bad9d3 into olbrich:master Nov 23, 2021
@olbrich olbrich added this to the Version 2.4 milestone Nov 24, 2021
@kreintjes kreintjes deleted the fix/class-method-calls branch November 24, 2021 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Minor add functionality in a backwards-compatible manner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants