-
Notifications
You must be signed in to change notification settings - Fork 493
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
added archival.tsv metadata block #10626
base: develop
Are you sure you want to change the base?
Conversation
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.
@Asbjoedt thanks for this, your first PR! I left some quick feedback.
archiveSubmitToArchivalAppraisal Yes yes 0 | ||
archiveSubmitToArchivalAppraisal No no 1 | ||
archiveSubmitToArchivalAppraisal Unknown unknown 2 |
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.
Is this the boolean? Yes, no, or unknown? As discussed here (and in person in Mexico):
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 PR only implements a controlled vocabulary list, because the boolean is not yet in the source code. The PR, #10480 , has grown stale, I think, but if possible when 10480 is further refined and merged, then the PR for this archival.tsv metadata block should adopt the boolean for this field specifically, because yes, as we discussed in Mexico, those fields you mention are exactly the use case why we need the boolean to properly handle the data type in Dataverse. My thought at the moment is to merge the archival.tsv and then when 10480 is merged send in a new PR to adopt it for archival.tsv.
what do you think?
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.
Yes, what you've done is similar (I think) to other quasi booleans we've defined in other metadata blocks, given the tools we have.
As of that boolean PR, I did at least leave a comment just now: #10480 (comment)
And yeah, especially if archival.tsv is marked as experimental in the guides, I imagine we could update it to take advantage of new functionality (like booleans) as they come along.
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.
Cool! Thanks for your comments.
The mapping of the boolean from frontend to backend could work like this:
Yes = true
No = false
Unknown = null
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.
I'll just leave this comment on the first file but we should add docs and a release note to this PR.
For docs, the new archival block should be listed under a future version of https://guides.dataverse.org/en/6.2/user/appendix.html#experimental-metadata
The release note can just say "A new, experimental archival metadata block has been added." The procedure is here: https://guides.dataverse.org/en/6.2/developers/version-control.html#writing-a-release-note-snippet
@pdurbin @Asbjoedt I pushed a PR Asbjoedt#1 to this PR for when you approve #11064 |
What this PR does / why we need it:
The PR adds an experimental "Archival" metadatablock. The purpose of the metadatablock is to enable repositories to register metadata relating to the potential archiving of the dataset at a depositor archive, whether that being your own institutional archive or an external archive i.e. a historical archive.
See related conversation on the Google Group: https://groups.google.com/g/dataverse-community/c/2wi1EnQwVOM
The original source of the "archival.tsv" file was created by University of Stuttgart. You can see the example here: https://github.com/izus-fokus/darus-metadata-blocks/blob/master/archival.tsv
I further expanded and edited on this source to what it is presented as now. I have asked University of Stuttgart for feedback on the 17th of May but have not heard anything. Therefore I send this PR in the hopes that Uni Stuttgart agrees with advancing this PR to get it into the source code and that we could use this PR for a review, where we would reach out to Uni Stuttgart and others for feedback before merging, and incorporating this feedback into the PR.
PS. This is my first Dataverse PR, I apologize for any formal requirements I may have missed. Secondly, I have not tested the PR, because I have not setup my computer for compiling and deploying the Dataverse source code yet. I still mean to do this.
Which issue(s) this PR closes:
Closes #
Special notes for your reviewer:
The PR only adds the metadata block. I'm not sure if there is more code needed to make the experimental metadata block discoverable by the sysadmin to enable it. If so, please help me with guidance.
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Maybe, a metadata block is viewable in the UI, but it does not change anything in the UI on how metadata blocks work.
Is there a release notes update needed for this change?:
I think so, yes.
Additional documentation: