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

🧹 Favor Sipity::Entity function over to_sipity_entity #2077

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Dec 19, 2023

The Sipity conversion methods are now explicit functions (much like the Array() function).

@jeremyf jeremyf added the minor-ver for release notes label Dec 19, 2023
Copy link
Member

@orangewolf orangewolf left a comment

Choose a reason for hiding this comment

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

to me, this is stepping backward. having as a method on the object allows the work class to handle the sipity entity in different ways depending on the objects class. if you use this method, the Sipity::Entity has to know about an unknown number to classes. Can you help me understand the api benifit of changing this?

@jeremyf
Copy link
Contributor Author

jeremyf commented Dec 19, 2023

@jeremyf jeremyf merged commit b01e6c5 into hyrax-5-upgrade Dec 19, 2023
2 of 4 checks passed
@jeremyf jeremyf deleted the fix-conversion-to-sipity branch December 19, 2023 22:18
@kirkkwang kirkkwang mentioned this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants