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

make it possible to add an author with a comment including e.g. an ORCID #69

Merged
merged 8 commits into from
Sep 19, 2018
Merged

make it possible to add an author with a comment including e.g. an ORCID #69

merged 8 commits into from
Sep 19, 2018

Conversation

maelle
Copy link
Contributor

@maelle maelle commented Sep 4, 2018

cf #65 (in preparation for the PR I intend to do for #63 )

One thing I wasn't sure about is whether you can add anything to the comment field, or whether there's a limited number of named strings that can go in there. I wasn't able to find that in "Writing R extensions" (where the ORCID support isn't documented yet anyway).

@codecov-io
Copy link

codecov-io commented Sep 4, 2018

Codecov Report

Merging #69 into master will decrease coverage by 1.22%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage     100%   98.77%   -1.23%     
==========================================
  Files          18       18              
  Lines        1131     1065      -66     
==========================================
- Hits         1131     1052      -79     
- Misses          0       13      +13
Impacted Files Coverage Δ
R/authors-at-r.R 98.43% <100%> (-1.57%) ⬇️
R/assertions.R 93.33% <75%> (-6.67%) ⬇️
R/built.R 87.5% <0%> (-12.5%) ⬇️
R/remotes.R 95.65% <0%> (-4.35%) ⬇️
R/description.R 97.51% <0%> (-2.49%) ⬇️
R/collate.R 98.18% <0%> (-1.82%) ⬇️
R/str.R 100% <0%> (ø) ⬆️
R/classes.R 100% <0%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f60833...1747a85. Read the comment docs.

@maelle
Copy link
Contributor Author

maelle commented Sep 4, 2018

I can't reproduce the Travis failure locally 🤔

@maelle
Copy link
Contributor Author

maelle commented Sep 4, 2018

Oh I know why it fails!

One of the tests I've added checks that one can add an ORCID. I check for the string resulting from format. In the newer R versions format formats the ORCID (to an URL), whereas it doesn't for older R versions.

I'll add some skipping.

@gaborcsardi gaborcsardi merged commit 9013f7d into r-lib:master Sep 19, 2018
@gaborcsardi
Copy link
Member

Thanks!

@maelle maelle deleted the multiple-comment branch September 19, 2018 12:58
llrs pushed a commit to llrs/desc that referenced this pull request Oct 9, 2018
…CID (r-lib#69)

* make it possible to add an author with a comment including an ORCID

* fix assertion

* fix on failure line

* fix assertion

* add test for NA assertion

* skip test on older R versions

* add test of failure

* typo!
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.

3 participants