-
Notifications
You must be signed in to change notification settings - Fork 26
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
Incorporate MIG sandbox edits. #52
Conversation
Putting field_place_published_country in its own module is maybe not the greatest idea. |
good to know: it's a bad idea to machine-name a vocabulary "form" because it seems to break the yaml. |
As you can see, the previous build worked, but we needed to move a field storage definition into controlled_access_terms for its pull request to be possible. That's why this will fail for the moment. |
There are risks in this PR when it comes to updating existing sites to this configuration, that may require further documentation:
|
Thanks @kayakr. Those are really important issues. I don't know if it's possible to update any content type (through features) when there's content in it. Like as soon as you touch field storage settings you're basically forcing a new site. I don't know if we've ever said that you should be able to update islandora defaults, or just have to look at the new setup and tweak yours to match. (ps. i hate it too) WRT alt title and subject (temporal), I think those might have been already committed to code, but for some reason the sandbox that we started with didn't have them (or didn't have them configured properly) so we re-added them? I'm following up to see why. For RDF changes, wouldn't a "full reindex" update existing data? |
I can confirm that the entire reason for the alternative title -> alt title thing in this PR is to change the cardinality from 1 to -1 (unlimited). I could refactor this PR to change the cardinality without creating a new field, but I don't know how that would go over on existing sites either. Or, again, if that is something we are concerned with. |
@rosiel Thanks for the response. Re "I don't know if we've ever said that you should be able to update islandora defaults", perhaps not, but if Islandora defaults changes and sites stay stuck on older versions and schemas IMO it will become increasingly challenging to incorporate functionality and features from the main islandora project. If the Islandora project provides no upgrade path for features in its purview then it imposes a burden on implementors that could mean that some sites fall behind and can't benefit from other community advancements. If there is a recommended approach to building on or with Islandora defaults, it would be good to have that documented clearly. If it is already documented, perhaps I missed that. Yes, a full reindex is the brute force approach to changing namespace values but for large collections it would be good to have tools providing a more precise data migration (probably out of scope for this PR but I expect the need will grow over time). Re changing the cardinality, it should be possible to achieve without needing a new field. https://www.drupal.org/node/2554097 describes a node_update hook for "changing cardinality of a field having existing data from single to multiple" that sounds like what is needed. We would need to adapt it to this usecase. |
@kayakr We're trying to make some sort of delineation between what Islandora (a.k.a. all of us) are responsible for maintaining and what each individual site is maintaining. We've got the That said, all those thoughts came at a time when it was mostly just programmers involved. This PR is an attempt to get contributions out of our Metadata Interest Group, and I guess is really highlighting the importance of the metadata best practices they are suggesting. If we think that's worthy enough for update hooks, we can go there and make sure they are included in the release. I'm not sure how we can do it without stomping on existing sites that have made significant changes, so there'd need to be some research / deep dives involved. Also, it need not be done by @rosiel, she actually took this task off of my plate as a favor and I don't want to burden her with a ton more work. Interested to hear what other @Islandora/8-x-committers think too. It's work. Do we think it's worthwhile? |
Wow, this is a lot of awesome work - and so good to have the input of metadata experts!
|
We've gone back and forth on this topic a number of times in Tech Calls over the past few years (and just now in response to this ticket). In the past year (or more?) we've generally settled on "once you install islandora_defaults, it's yours" as @dannylamb described. We tried to use Features to support selectively incorporating updates, but the configs are so intertwined that it doesn't always work well and, of course, some things can't be changed without an update hook. Adding update hooks will cause Drupal to constantly nag sites that don't want specific changes to run them. I think we need to get to the point where people think of islandora_defaults is search_api_solr_defaults.... you install it once when you create your site to get your initial config, but then you disable it and the config stays behind. It is then yours. Defaults would represent the community's best guess as to what a new site wants but sites need to make changes on their own (either via the UI, config sync, or their own update hooks). The community can try to help through documentation ("Here is how to get the cool new thing!") and conversations on Slack or email. I took a quick glance at islandora_defaults and the main blocker from getting to that point, where you can uninstall islandora_defaults, is
TL;DR: core islandora module config changes need update hooks to keep existing sites working while islandora_defaults config changes should be left alone to avoid forcing changes on sites that don't want them. |
😭 I can't find where to restart tests now that we've changed to using Github CI! |
I tested this with ISLE quite easily. For posterity, here's what I did
This resulted in a new site with no content, the latest code and configuration from this PR. |
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 MIG held a sprint to audit the metadata profile. They made changes on a sandbox that were tracked in this document:
https://docs.google.com/document/d/18XChGOCWm_bT_S-UsfOl8l9xnsioF-L8sdsBXMQcqXU/edit#heading=h.9bn0ps9gm8x9
Diff between the altered sandbox and a clean sandbox was analyzed line-by-line, and relevant changes to Defaults were pulled in here. Tracking document: https://docs.google.com/spreadsheets/d/15speAh8yQ0-Cuww08efOUGjqw2v6KBXD2sUji5X47zQ/edit#gid=0
What does this Pull Request do?
Pulls in changes that applied to Islandora Defaults.
What's new?
Lots of changes to field labels and descriptions.
A few new fields
A few new taxonomy vocabularies:
Edits to the genre and language vocabularies will be in another PR to controlled access terms.
How should this be tested?
Does it install as cleanly as Defaults?
Additional Notes:
This process sucks.
Interested parties
Tag (@ mention) interested parties or, if unsure, @Islandora/8-x-committers