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

Update DOI and LICENSE for CRAN submission #54

Merged
merged 9 commits into from
Mar 11, 2020

Conversation

hongyuanjia
Copy link
Contributor

First submission try, still got a NOTE. CRAN maintainer replied:

Thanks, we see:

   License components with restrictions and base license permitting such:
     MIT + file LICENSE
   File 'LICENSE':
     Year: 2018-2020
     COPYRIGHT HOLDER: The PsychroLib Contributors

Please use YEAR: 2018-2020

   Found the following (possibly) invalid DOIs:
     DOI: doi.org/10.21105/joss.01137
       From: inst/CITATION
       Message: Invalid DOI

The DOI used in the doi markup is only 10.21105/joss.01137

Please fix and resubmit.

According CRAN policy, year field in MIT LICENSE should start as YEAR not Year. Also, DOI prefix should be removed.

This PR modified src/r/tools/deploy.R and src/r/inst/CITATION accordingly.

According CRAN policy, year field in MIT LICENSE should start as `YEAR` not `Year`. Also, DOI prefix should be removed.
@hongyuanjia hongyuanjia requested a review from dmey January 23, 2020 10:50
@dmey
Copy link
Contributor

dmey commented Jan 23, 2020

I'd prefer to keep master clean -- are you OK with waiting with the merge until CRAN accepts it? Just in case there are more changes to do...

@hongyuanjia
Copy link
Contributor Author

Sure. Make sense. Will try second submission now.

@codecov-io
Copy link

codecov-io commented Jan 23, 2020

Codecov Report

Merging #54 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #54   +/-   ##
=======================================
  Coverage   88.42%   88.42%           
=======================================
  Files           3        3           
  Lines         380      380           
=======================================
  Hits          336      336           
  Misses         44       44

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 49a2baa...d511e68. Read the comment docs.

@hongyuanjia
Copy link
Contributor Author

Finally get reply from CRAN maintainers (normally they will reply in a day):

Thanks,

Please omit the redundant "R" from the begin of the description.

"[...]Where copyrights are held by an entity other than the package 
authors, this should preferably be indicated via ‘cph’ roles in the 
‘Authors@R’ field, or using a ‘Copyright’ field (if necessary referring 
to an inst/COPYRIGHTS file). [...]"
e.g.: The PsychroLib Contributors, ASHRAE Handbook
Please explain in the submission comments what you did about this issue.

Please add references describing the methods in your package also to the 
description field of your DESCRIPTION file in the form
authors (year) <doi:...>
authors (year) <arXiv:...>
authors (year, ISBN:...)
or only if none those are available:  <https:...>
with no space after 'doi:', 'arXiv:', 'https:' and angle brackets for 
auto-linking.
(If you want to add a title as well please put it in quotes: "Title")

Please fix and resubmit.

For 1 and 3, it's easy to fix.

For the copyrights, it seems that there is no specific one acts as the cph role. I think it may make more clear to use an individual inst/COPYRIGHT file which contains contents like:

  1. Hongyuan Jia and Jason Banfelder for R implementation.
  2. D. Thevenard and D. Meyer for the core library implementations.
  3. Equations and coefficients published ASHRAE Handbook — Fundamentals, Chapter 1 Copyright (c) 2017 ASHRAE Handbook — Fundamentals (https://www.ashrae.org)

What do you think @dmey @DJGosnell ?

@dmey
Copy link
Contributor

dmey commented Feb 3, 2020

this should preferably be indicated via ‘cph’ roles

this looks more like a preference than anything else -- In any case, let's try to keep this simple -- I would simply use what's already done at https://github.com/rstudio/rmarkdown/blob/master/DESCRIPTION#L36-L37. Specifically, I would change it to:

  person(family = "The PsychroLib Contributors", role = c("ctb", "cph"),
         comment = "Authors listed at https://github.com/psychrometrics/psychrolib/graphs/contributors"),

Then add you as maintainer

Maintainer: Hongyuan Jia <email>

src/r/DESCRIPTION Show resolved Hide resolved
@hongyuanjia
Copy link
Contributor Author

That is a feasible way. My only concern is whether ASHARE should be put as an author.

src/r/DESCRIPTION Outdated Show resolved Hide resolved
Co-Authored-By: dmey <dmey@users.noreply.github.com>
@dmey
Copy link
Contributor

dmey commented Feb 3, 2020

That is a feasible way. My only concern is whether ASHARE should be put as an author.

OK, how about something like?

  person(family = "The PsychroLib Contributors", role = c("ctb", "cph"),
         comment = "Authors listed at https://github.com/psychrometrics/psychrolib/graphs/contributors"),
  person(family = "ASHRAE", role = c("cph")
         comment = "Copyright 2017 ASHRAE Handbook — Fundamentals (https://www.ashrae.org) for equations and coefficients published ASHRAE Handbook — Fundamentals Chapter 1."),

@hongyuanjia
Copy link
Contributor Author

That is a feasible way. My only concern is whether ASHARE should be put as an author.

OK, how about something like?

  person(family = "The PsychroLib Contributors", role = c("ctb", "cph"),
         comment = "Authors listed at https://github.com/psychrometrics/psychrolib/graphs/contributors"),
  person(family = "ASHRAE", role = c("cph")
         comment = "Copyright 2017 ASHRAE Handbook — Fundamentals (https://www.ashrae.org) for equations and coefficients published ASHRAE Handbook — Fundamentals Chapter 1."),

Seems like this did not work. R CMD check directly fails. Probably we could keep the original style and also add ASHRAE?

E  checking DESCRIPTION meta-information ...
   Authors@R field gives no person with maintainer role, valid email
   address and non-empty name.

   See section 'The DESCRIPTION file' in the 'Writing R Extensions'
   manual.

@dmey
Copy link
Contributor

dmey commented Feb 3, 2020

Can you try simply by adding your email before role under "The PsychroLib Contributors"

@hongyuanjia
Copy link
Contributor Author

No lock. It seems that the first "person" field should always have an non-empty name.

@dmey
Copy link
Contributor

dmey commented Feb 3, 2020

No lock. It seems that the first "person" field should always have an non-empty name.

How about you just add yourself as the first, rest should be fine as is?

@hongyuanjia
Copy link
Contributor Author

How about you just add yourself as the first, rest should be fine as is?

Worked!

@hongyuanjia
Copy link
Contributor Author

@dmey Latest reply from CRAN maintainer:

Thanks,


On 04.02.20 03:07, CRAN submission wrote:
> [This was generated from CRAN.R-project.org/submit.html]
> 
> The following package was uploaded to CRAN:
> ===========================================
> 
> Package Information:
> Package: psychrolib
> Version: 2.4.0
> Title: Psychrometric Properties of Moist and Dry Air
> Author(s): Hongyuan Jia [aut, cre]
>    (<https://orcid.org/0000-0002-0075-8183>), The PsychroLib
>    Contributors [ctb, cph] (Authors listed at
>    https://github.com/psychrometrics/psychrolib/graphs/contributors),

Authors and copyright holder should be part of the package.

If there are many authors and/or copyright holders, follow the advise 
and Writing R  Extensions and provide a file that lists all of them (one 
for authors and one for copyright hodlers), but they have to be part of 
the package as explained in the manual.

Please fix and resubmit.

According to Writing R Extensions:

The mandatory ‘Author’ field describes who wrote the package. It is a plain text field intended for human readers, but not for automatic processing (such as extracting the email addresses of all listed contributors: for that use ‘Authors@R’). Note that all significant contributors must be included: if you wrote an R wrapper for the work of others included in the src directory, you are not the sole (and maybe not even the main) author.

It seems that we have to list all authors like rmarkdown in a separate file and include it in the package under inst/ folder, instead of linking to a web file.

Will update and resubmit

# |<----  Using a Maximum Of 50 Characters  ---->|


# Explain why this change is being made
# |<----   Try To Limit Each Line to a Maximum Of 72 Characters   ---->|

# Provide links or keys to any relevant tickets, articles or other resources
# Example: Github issue psychrometrics#23

# --- COMMIT END ---
# Type can be 
#    feat (new feature)
#    fix (bug fix)
#    docs (changes to documentation)
#    style (formatting, missing semi colons, etc; no code change)
#    refactor (refactoring production code)
#    test (adding missing tests, refactoring tests; no production code change)
#    chore (updating grunt tasks etc; no production code change)
# --------------------
# Remember to
#    Separate subject from body with a blank line
#    Limit the subject line to 50 characters
#    Capitalize the subject line
#    Do not end the subject line with a period
#    Use the imperative mood in the subject line
#    Wrap the body at 72 characters
#    Use the body to explain what and why vs. how
#    Can use multiple lines with "-" for bullet points in body
# --------------------
# For more information about this template, check out
# https://gist.github.com/adeekshith/cd4c95a064977cdc6c50
@hongyuanjia
Copy link
Contributor Author

hongyuanjia commented Feb 27, 2020

@dmey
Copy link
Contributor

dmey commented Feb 27, 2020

Great! But I am bit confused as the binaries are not available -- my understanding when we discussed the PR about having C++ source was that the package on CRAN would not require the user to compile psychrolib from source. Can you clarify this please?

@hongyuanjia
Copy link
Contributor Author

It usually takes few days for CRAN to compile the package for Windows and MacOS. So just wait.

@hongyuanjia
Copy link
Contributor Author

@dmey I believe this PR is ready to be merged. Right now Windows and macOS binaries have been built. You can check that by installing the package via install.packages("psychrolib").

@dmey
Copy link
Contributor

dmey commented Mar 9, 2020

Thanks for letting me know -- I will try to create the release and merge this in the next couple of days.

@dmey dmey self-requested a review March 11, 2020 12:46
@dmey dmey merged commit ad90022 into psychrometrics:master Mar 11, 2020
@hongyuanjia hongyuanjia deleted the hotfix/CRAN_submission branch April 8, 2020 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants