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 method_missing over a raft of delegates #6763

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Mar 29, 2024

It's like we're flooding a political convention with delegates. Instead of having all of those delegate methods, let's just implement method missing and pass things along to the SolrDocument.

The benefit is that as we incorporate dynamic metadata, we don't need to keep on adding delegate :this_property, to: :solr_document.

@samvera/hyrax-code-reviewers

@jeremyf jeremyf force-pushed the favor-method-missing-over-all-that-delegation branch from a1517e1 to 6951cd0 Compare March 29, 2024 17:59
@jeremyf jeremyf requested a review from dlpierce March 29, 2024 18:03
@jeremyf jeremyf force-pushed the favor-method-missing-over-all-that-delegation branch from 6951cd0 to 1a2c4b8 Compare March 29, 2024 18:26
It's like we're flooding a political convention with delegates.  Instead
of having all of those delegate methods, let's just implement method
missing and pass things along to the `SolrDocument`.

The benefit is that as we incorporate dynamic metadata, we don't need to
keep on adding `delegate :this_property, to: :solr_document`.
@jeremyf jeremyf force-pushed the favor-method-missing-over-all-that-delegation branch from 1a2c4b8 to a3d4a8f Compare March 29, 2024 18:49
@dlpierce
Copy link
Contributor

I like this. I was surprised to still need to customize the presenter (and other files) when we have the metadata schema yaml files in place. However, I'm going to wait until after 5.0.1 to merge this.

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.

3 participants