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

DS-3908 Metadata patch support #46

Merged
merged 6 commits into from Feb 28, 2019

Conversation

Projects
None yet
4 participants
@cwilper
Copy link
Contributor

cwilper commented Jan 2, 2019

This adds contract docs for metadata PATCH support.

https://jira.duraspace.org/browse/DS-3908

It mainly just adds this document and links to it from appropriate places: https://github.com/DSpace/Rest7Contract/blob/0f04637d0ca16e42ad1fcce251e17db49b0fb701/metadata-patch.md

REST implementation PR: DSpace/DSpace#2313

@abollini
Copy link
Member

abollini left a comment

The general concept is fine, I have just added some minor suggestions (see inline comments)

Show resolved Hide resolved metadata-patch.md Outdated
Show resolved Hide resolved metadata-patch.md
Show resolved Hide resolved metadata-patch.md
@tdonohue

This comment has been minimized.

Copy link
Member

tdonohue commented Feb 5, 2019

@cwilper : I gave this a quick review today too. I think this looks mostly fine to me. I don't have major suggestions for improvement, but I agree with @abollini that we should minimally better align the PATCH docs here with the PATCH docs for workspaceitem. Ideally, it'd be nice to consolidate the two (if there's a way to clearly do so). But, I don't think that's a requirement of this PR, especially since I'm not sure myself if these can be easily consolidated into a single section on PATCH operations for metadata (in general).

By the way, this PR currently has merge conflicts that need resolving.

@cwilper cwilper force-pushed the atmire:DS-3908_Metadata_patch_support branch from 2b27aae to 586df29 Feb 21, 2019

@cwilper

This comment has been minimized.

Copy link
Contributor Author

cwilper commented Feb 21, 2019

Thanks @abollini and @tdonohue , I have brought this PR back up to date with master and addressed the comments to date.

@cwilper cwilper force-pushed the atmire:DS-3908_Metadata_patch_support branch 2 times, most recently from 0f04637 to 3b4b4f7 Feb 21, 2019

@paulo-graca
Copy link

paulo-graca left a comment

Good job!

@tdonohue
Copy link
Member

tdonohue left a comment

👍 This looks good to me now. I did notice one (very minor) typo -- see inline comment.

Show resolved Hide resolved communities.md Outdated

@cwilper cwilper force-pushed the atmire:DS-3908_Metadata_patch_support branch from 3b4b4f7 to a7b477b Feb 28, 2019

I've double-checked this morning, and all of @abollini's prior suggestions have been implemented

@tdonohue

This comment has been minimized.

Copy link
Member

tdonohue commented Feb 28, 2019

In the DSpace 7 meeting today, we noted this is already at +2, and @abollini said we are welcome to merge as-is. If further minor changes are necessary, we can clean them up in a future PR. Thanks @cwilper!

@tdonohue tdonohue merged commit 4c721f3 into DSpace:master Feb 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.