VLucet / rgovcan

Easy access to the Canadian Open Government Portal
https://vlucet.github.io/rgovcan/
21 stars 4 forks source link

Release rgovcan on CRAN #13

Closed VLucet closed 3 years ago

VLucet commented 3 years ago

Before getting the release ready:

First release:

Prepare for release:

Submit to CRAN:

Wait for CRAN...

VLucet commented 3 years ago

@KevCaz I'm finally getting around to doing this, as promised! Sorry for the delay, pandemic etc..

KevCaz commented 3 years ago

Thanks for the update @VLucet , just happy to help further if I can, as this is a side project I'm expecting you to work on it at your own pace, when time/pandemic permits. I guess my point is that you don't have to be sorry :smile: !

VLucet commented 3 years ago

I know! Just needed to acknowledge the need to move things forward haha Code coverage is low so I'll be focusing on increasing that before anything.

KevCaz commented 3 years ago

Let me know if you need some help!

VLucet commented 3 years ago

Thanks so much @KevCaz ! A few things:

PS: for future reference, note that the use of testhat::context() has been superseded (the context is the name of the test file now).

KevCaz commented 3 years ago

Could you explain to me what was going on there?

I guess this is related to #11.

In addition, I have removed the need for the github version of crul because the code is running fine with the CRAN version, but i know its because of the suppresswarning statements in the code

That's good, so no problem and the tests commented out should work now, I guess.

Could you remind me why the use the gfm variant in the readme?

To use all the features included in this Markdown flavor. That said, that may now be the default behavior of rmarkdown!

VLucet commented 3 years ago

Thanks, tests are passing now, except for this line. And I have no clue why (maybe you can be smarter than me and figure it out?). Test coverage is satisfactory now so I'll ne moving on to other checks.

VLucet commented 3 years ago

Also using rhub::check_for_cran() gives out url issues I don't seem to be able to fix.

   Found the following (possibly) invalid URLs:
     URL: https://open.canada.ca/data/en
       From: man/govcan_setup.Rd
       Status: Error
            schannel: next InitializeSecurityContext failed: SEC_E_ILLEGAL_MESSAGE (0x80090326) - This error usually occurs when a fatal SSL/TLS alert is received (e.g. handshake failed).
       Message: libcurl error code 35:
     URL: https://open.canada.ca/en
             man/govcan_get_record.Rd
       From: DESCRIPTION
       Status: Error
       Message: libcurl error code 35:

            schannel: next InitializeSecurityContext failed: SEC_E_ILLEGAL_MESSAGE (0x80090326) - This error usually occurs when a fatal SSL/TLS alert is received (e.g. handshake failed).

EDIT: after some research I think this is ok, I can explain in the cran comments if necessary.

VLucet commented 3 years ago

I tried to submit! fingers crossed.

KevCaz commented 3 years ago

I wouldn't be surprised if they ask you some tiny things. For future releases, I would consider testing the package with vcr:

VLucet commented 3 years ago

Thanks, will take a look! I may have gotten a little too impatient to submit... oh well, we'll see

KevCaz commented 3 years ago

Hahaha, we'll see!

VLucet commented 3 years ago

Comments back, I apparently forgot to document properly the constructors. That's a stupid thing to have missed but at least it's easy to fix.

Please add \value to .Rd files regarding exported methods and explain
the functions results in the documentation. Please write about the
structure of the output (class) and also what the output means. (If a
function does not return a value, please document that too, e.g.
\value{No return value, called for side effects} or similar)
Missing Rd-tags:
      new_ckan_package_stack.Rd: \value
      new_ckan_resource_stack.Rd: \value

Resubmitted.

KevCaz commented 3 years ago

I can do a quick check later today if you want me to!

VLucet commented 3 years ago

@KevCaz I have resubmitted a couple time times and I am having issues with which you might be able to help.

  1. 1st submission => problem of docs, fixed
  2. 2nd submission : this email, which I took for an wrror on the side of opencan but maybe I was mistaken
    
    Dear maintainer,

package rgovcan_1.0.1.tar.gz does not pass the incoming checks automatically, please see the following pre-tests: Windows: https://win-builder.r-project.org/incoming_pretest/rgovcan_1.0.1_20210312_192154/Windows/00check.log Status: 1 NOTE Debian: https://win-builder.r-project.org/incoming_pretest/rgovcan_1.0.1_20210312_192154/Debian/00check.log Status: 1 ERROR, 1 NOTE

3. 3rd submission, got this email:

Apparently this is not safe for internet resoursc eproblems, please re-read the CRAN plicies and fix it.

Best, Uwe Ligges

KevCaz commented 3 years ago

I'm guessing Uwe is referring to

Packages which use Internet resources should fail gracefully with an informative message if the resource is not available or has changed (and not give a check warning nor error).

in https://cran.r-project.org/web/packages/policies.html.

Honestly, it is worth re-doing the tests with vcr and avoid all docs/examples that actually query the API. I understand that it may sound sub-optimal, but there are too much you cannot control (we experienced various issues with rmangal). Also, it might be worth avoiding sending too many pkg submissions the same day, hahaha.

VLucet commented 3 years ago

Also, it might be worth avoiding sending too many pkg submissions the same day, hahaha.

100% agree, I was pushing my luck...

it is worth re-doing the tests with vcr

Okay, I will take a serious look at it then.

avoid all docs/examples that actually query the API

do you mean wrapping these in dontrun?

KevCaz commented 3 years ago

do you mean wrapping these in dontrun?

Yep. We'll see what they say, but when you resubmit, in the cran-comments.md file, you can mention that you do this to avoid querying the web API from CRAN servers.

VLucet commented 3 years ago

Okay, picking up the work on this. I would appreciate if you could help me catch up on what was decided. Should we focus on using vcr in the end or should I simply try to submit with dontrun examples and skip_on_cran tests?

VLucet commented 3 years ago

Maybe the best thing for now is to submit with examples and tests skipped and move on to vcr later.

VLucet commented 3 years ago

Accepted on CRAN 🥳