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

MIG changes May 2021 #69

Merged
merged 3 commits into from
Jul 13, 2021
Merged

MIG changes May 2021 #69

merged 3 commits into from
Jul 13, 2021

Conversation

rosiel
Copy link
Member

@rosiel rosiel commented May 21, 2021

GitHub Issue: (link)

Companion to: Islandora/islandora_defaults#52

What does this Pull Request do?

Updates names/descriptions/fields on taxonomy vocabularies per MIG recommendations.

What's new?

Rename and redescribe fields.

How should this be tested?

Does it build (requirements aren't messed up)?

Additional Notes:

Interested parties

Hey @Islandora/8-x-committers
New interface upgrades for the release!

@dannylamb
Copy link
Contributor

Is the change here to rebrand field_authority_links as field_authority_sources or are they supposed to live side by side?

@kspurgin
Copy link
Contributor

I am not seeing a record of changing this field name as part of the MIG work documented here

Looking around a bit more to see if anything jogs my memory.

@kspurgin
Copy link
Contributor

Ok, I have developed a theory @dannylamb

@rosiel had mentioned we did work based on a Sandbox that was running the last release (from 2020-01-05), so what we based our sprint on didn't include any contributions that had been made in the meantime.

Like Natkeeran's creation of field.field.taxonomy_term.language.field_authority_link.yml, merged 2020-01-13.

So we duplicated work that had already been done since that release and introduced this minor inconsistency in how fields were set up because we were not seeing the precedent that has been merged into the code base since 2020-01-05.

It looks to me like all the machine names of the fields with authority_link field type should be authority_link

I am betting I typed in the field label "Authority Sources", when creating the "New" genre taxonomy in defaults, since that is what the label for these fields is in the other taxonomies, and it auto-generated the authority_sources machine name.

@kspurgin
Copy link
Contributor

I'm working on a PR to Rosie's fork that can be used here if @rosiel deems it fit to merge

@kspurgin kspurgin self-requested a review June 30, 2021 18:51
Copy link
Contributor

@kspurgin kspurgin left a comment

Choose a reason for hiding this comment

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

This needs changes, which I will submit as a PR to @rosiel 's fork.

MIG did not have the latest `controlled-access-terms` applied to the
sandbox on which we were doing our work, so some things got a bit
duplicated inexactly.
reconcile inconsistencies introduced by MIG Sprint based on old code
@dannylamb
Copy link
Contributor

Awesome @kspurgin and @rosiel . Merging this in. I'll test it on a fresh instance along with the islandora_defaults changes. If it's good, this is in the release.

FINALLY! 🚀

@dannylamb dannylamb merged commit 917d66b into Islandora:8.x-1.x Jul 13, 2021
@rosiel rosiel deleted the 8.x-1.x branch July 21, 2022 16:25
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.

None yet

3 participants