Skip to content

Fixed #1375 Author Identifier Additions#6358

Merged
kcondon merged 1 commit intoIQSS:developfrom
mderuijter:1375-author-identifier-additions
Nov 12, 2019
Merged

Fixed #1375 Author Identifier Additions#6358
kcondon merged 1 commit intoIQSS:developfrom
mderuijter:1375-author-identifier-additions

Conversation

@mderuijter
Copy link
Contributor

@mderuijter mderuijter commented Nov 12, 2019

Related Issues

Pull Request Checklist

@coveralls
Copy link

Coverage Status

Coverage remained the same at 19.464% when pulling c9618d5 on mderuijter:1375-author-identifier-additions into 41acae1 on IQSS:develop.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I left some comments in my review. Thanks!

authorIdentifierScheme DAI 5
authorIdentifierScheme ResearcherID 6
authorIdentifierScheme ScopusID 7
language Abkhaz 0
Copy link
Member

@pdurbin pdurbin Nov 12, 2019

Choose a reason for hiding this comment

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

I got curious about why the line for "Abkhaz" changed and it looks like the trailing tabs were removed:

Screen Shot 2019-11-12 at 6 45 13 AM

It looks like the new lines for authorIdentifierScheme don't have trailing tabs either. I assume this is fine.

It would be nice to add a file under "doc/release-notes" to explain to installations what they should do to add the new values. My understanding is that they should re-run the curl command for the citation.tsv file. This process of writing release notes is described at http://guides.dataverse.org/en/4.17/developers/making-releases.html#write-release-notes

Finally, I believe this pull request closes #3169 so the description should probably be change to that (instead of "related"). On his way out the door, @djbrooke indicated that he might want to let @Venki18 make the change himself during Danny's visit to NTU but I think we should just leave "ResearcherID" in. More contributions can be made in the future. 😄

Copy link
Member

Choose a reason for hiding this comment

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

I believe this pull request closes #3169 so the description should probably be change to that (instead of "related")

I just made this change.

Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to add a file under "doc/release-notes" to explain to installations what they should do to add the new values. My understanding is that they should re-run the curl command for the citation.tsv file.

@kcondon I just moved this pull request to QA without the release note mentioned above. If you want me to hop on @mderuijter 's branch and add it (or add it after merging) please let me know.

@pdurbin
Copy link
Member

pdurbin commented Nov 12, 2019

I just thought I'd mention that thanks to out new "spin up pull requests on EC2" automation (thank you, @donsizemore !) we can see the new author identifiers like in the screenshot below at http://ec2-3-90-251-34.compute-1.amazonaws.com

Screen Shot 2019-11-12 at 9 54 25 AM

@kcondon kcondon self-assigned this Nov 12, 2019
@kcondon
Copy link
Contributor

kcondon commented Nov 12, 2019

Tested, looks ok to merge. @pdurbin maybe we can just update Danny's pr with the instructions?

@kcondon kcondon merged commit 1707c04 into IQSS:develop Nov 12, 2019
@pdurbin
Copy link
Member

pdurbin commented Nov 12, 2019

maybe we can just update Danny's pr with the instructions?

Sure, I just added ee10648. Please take a look, edit etc.

@kcondon
Copy link
Contributor

kcondon commented Nov 12, 2019

@pdurbin looks good, thanks

@djbrooke djbrooke added this to the 4.18 milestone Nov 14, 2019
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.

authorIdentifierScheme : Add "ResearcherID" as one of the option Author/User Identifiers: Suggested Additions

5 participants