Merged
Conversation
sdruskat
approved these changes
May 15, 2023
Contributor
sdruskat
left a comment
There was a problem hiding this comment.
Thanks @led02!
Looks good to me so far.
I've changed to use the default YAML loader instead of CLoader in b88367b and re-worked CFF updating quite a bit in 51fd2bb. Please review carefully!
Notes on CFF updating:
- CFF doesn't (yet) have DataCite-like relations for identifiers, so this will append rather than re-write the new identifier to a (perhaps already existing) list of identifiers. I refrained from doing too much string diffing to detect previous versions, etc. This can be re-worked and made much better once CFF has the relations between identifiers implemented.
- Like I said, identifier overwrites are switched off now.
- The identifier description has been updated to include the version number from the deposit metadata, so that human users (for whom the description is) know what they are looking at. Ideally, we'd remove identifiers for older versions, but this isn't easily doable now without extra calls to the Invenio API.
| cff = yaml.load(open('CITATION.cff', 'r'), yaml.CLoader) | ||
| cff['identifiers'] = [ | ||
| { | ||
| 'description': "Generated by hermes after deposition.", |
Contributor
There was a problem hiding this comment.
Suggested change
| 'description': "Generated by hermes after deposition.", | |
| 'description': f"DOI for the published version {deposition['metadata']['version']} [generated by hermes]", |
polish: Better describe the value of the identifier, use square brackets for easier data mining later.
Contributor
|
@zyzzyxdonta Please feel free to give this a quick review as well. |
Co-authored-by: Stephan Druskat <sdruskat@users.noreply.github.com>
zyzzyxdonta
reviewed
May 16, 2023
Co-authored-by: David Pape <d.pape@hzdr.de>
Co-authored-by: David Pape <d.pape@hzdr.de>
poikilotherm
approved these changes
May 22, 2023
Member
poikilotherm
left a comment
There was a problem hiding this comment.
We have seen in the showcase testing this addition is very valuable and works without flaws so far. Ship it! ![]()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.