-
Notifications
You must be signed in to change notification settings - Fork 256
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
Extract reflect_fields method to the repository #1914
Conversation
repository = repository_class.new(self) | ||
repository.send_and_receive('admin/luke', params: { fl: '*', 'json.nl' => 'map' })['fields'] | ||
end | ||
repository = repository_class.new(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you safely remove the condition around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests seem to pass. I can think of no reason why we can't require repository implementations to implement this interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a problem; I don't remember what the luke response looks like, but if we think something similar will work cross-repository, I don't see a problem with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, overall. Have a question about removing a condition check.
end | ||
end | ||
end | ||
|
||
def luke_fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename this method?
This allows the repository to implement reflect_fields and allows us to remove a conditional.
This allows the repository to implement reflect_fields and allows us to
remove a conditional.