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

Fixed AST::Node#updated to always return a copy. #25

Merged

Conversation

iliabylich
Copy link
Collaborator

@iliabylich iliabylich commented Jun 10, 2020

Currently it relies on a call chain #initialize -> #freeze that returns self.
However, overriding constructor in a custom subclass and returning something else breaks updated.

Included a test case. Without my fix updated returns nil.

@whitequark
Copy link
Owner

Could you split code and CI changes in separate commits? Other than that LGTM

@coveralls
Copy link

coveralls commented Jun 10, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 239014d on iliabylich:fix-updated-method-for-subclasses into 05d2321 on whitequark:master.

Currently it relies on a call chain #initialize -> #freeze that returns `self`.
However, overriding constructor in a custom subclass and returning something else breaks `updated`.
@iliabylich iliabylich force-pushed the fix-updated-method-for-subclasses branch from 0f9e92d to 239014d Compare June 10, 2020 17:04
@iliabylich iliabylich merged commit 9a0bfdb into whitequark:master Jun 10, 2020
@iliabylich iliabylich deleted the fix-updated-method-for-subclasses branch June 10, 2020 17:10
@iliabylich
Copy link
Collaborator Author

@whitequark What is the versioning strategy here? Should the next version be 2.4.1?

@whitequark
Copy link
Owner

That's a bugfix AFAIU, so yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants