-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add configurability to how we extract v3 manifest properties from a record #76
Conversation
expose the configuration, add tests Co-authored-by: Jeremy Friesen <[email protected]>
Seems my attempt at adding a Co-author didn't work.. This was from pairing with Jeremy |
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.
This is interesting! I think this could work.
The documentation and backwards-compatibility are great!
Before merging I think it would be good to update the README and to use the new pattern in the v2 manifest builder here:
iiif_manifest/lib/iiif_manifest/manifest_builder/record_property_builder.rb
Lines 13 to 14 in 0c014de
manifest.label = record.to_s | |
manifest.description = record.description |
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.
This looks great! Thanks for adding this useful new feature!
Prior to this commit:
The
description
,label
,rights
, andhomepage
properties were not configurable and relied on the methods supplied by the#record_property_builder
. This made it difficult if an application uses the#abstract
instead of#description
to populate thesummary
property.With this commit:
The above mentioned properties are configurable and fall back to its default when there is no configuration.