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

Move license attribute row from _metadata partial to _attribute_rows … #3153

Merged
merged 1 commit into from
Nov 12, 2018

Conversation

adamjarling
Copy link
Member

…partial

Fixes #394

This PR attempts to fix "Move rights rows from _metadata to _attribute_rows", but see issue #394 for needed clarification of what's the acceptance criteria for this issue. It's not clear to me.

@samvera/hyrax-code-reviewers

@cjcolvar
Copy link
Member

We actually made this same change in our hyrax app so I think this is a good change.

@no-reply
Copy link
Contributor

I'd like to get this in in the near future, but I have a concern:

Many apps override _attribute_rows, but not _metadata, which means this change will remove license from their runtime copies of _metadata, but not add it to _attribute_rows.

I'd like to hold this for the time being. I've put it on the agenda for Samvera Tech next week.

@RudyOnRails
Copy link
Contributor

Looks like this agenda item got lost in the setup of the next week's agenda: https://wiki.duraspace.org/display/samvera/Copy+of+Samvera+Tech+Call+2018-07-25

I have added it to the new agenda for next week..

@no-reply
Copy link
Contributor

There is consensus on Samvera Tech is that this would constitute a breaking change and require bumping the version to 3.0.

Since it's a small change, I'm inclined to hold it until we have other API changes in the pipeline.

@stale
Copy link

stale bot commented Oct 11, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 11, 2018
@no-reply
Copy link
Contributor

Considering this along with #3121 as motivating 3.0.0 soon.

Requesting feedback, especially from @vantuyls @samvera/hyrax-code-reviewers @samvera/hyrax-repo-managers

@stale stale bot removed the stale label Oct 13, 2018
@no-reply no-reply added this to the 3.x series milestone Oct 15, 2018
@adamjarling
Copy link
Member Author

@no-reply What do you suggest for this? Ok to merge now?

@adamjarling adamjarling self-assigned this Oct 29, 2018
Copy link
Contributor

@no-reply no-reply left a comment

Choose a reason for hiding this comment

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

Yay!

I started a draft release for v3.0.0-beta1 at https://github.com/samvera/hyrax/releases/tag/untagged-2151e64eb6ad0bfb86e9

@no-reply no-reply merged commit 84272a9 into master Nov 12, 2018
@no-reply no-reply deleted the 394-move-rights-rows-to-attribute-rows branch November 12, 2018 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move rights rows from _metadata to _attribute_rows
4 participants