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

Multiple minor CRAN check-related updates #17

Merged
merged 3 commits into from
Nov 27, 2020

Conversation

jranke
Copy link
Contributor

@jranke jranke commented Nov 27, 2020

Just trying to lend a hand here. And checking a package is really fun using rcc.r!

This just adds .github to .Rbuildignore

I also get another note:

N  checking dependencies in R code (340ms)
   Unexported object imported by a ':::' call: ‘Rcpp:::RcppCxxFlags’
     See the note in ?`:::` about the use of this operator.

I don't understand why this does not show up on the CRAN check page. Anyways I would leave this one for you.

@eddelbuettel
Copy link
Owner

Hm, I usually catch these at home too but I was really busy with other yesterday. Let me take a look. (And yes, rcc.r is fun. Maybe it should default to setting -c as well...)

@eddelbuettel
Copy link
Owner

I should have caught the .github/ addition as that has of course me before when I switched on GitHub Actions...

I do not see the warning about :::. Neither do GitHub or Travis (which I had briefly turned off yesterday).

We probably also want a .gitignore so I will just add this to this PR and merge then.

@jranke
Copy link
Contributor Author

jranke commented Nov 27, 2020

Cool, I did not know you can push to a PR 👍

@eddelbuettel
Copy link
Owner

The triple colon is clearly there ... and almost surely not needed anymore (there was a rewrite of Rcpp many, many moons ago). For continuity's sake I will just make that paste("-I", system.file("include", package="Rcpp"), sep="") which does the same without verboten symbols.

@eddelbuettel
Copy link
Owner

(Re the pushing to your branch: Yeah. It's a new-ish feature of GH, maybe one or two years old, with a default-on toggle when you create a PR -- look around next time ;-) Also, it is magic how the Emacs magit mode just checks out a branch by PR number only. Could. Not. Be. Easier.)

@jranke
Copy link
Contributor Author

jranke commented Nov 27, 2020

Oh, seems I am really missing out using vim and the fugitive plugin (which I think are really cool nonetheless). But I guess we should not turn this into a vi vs. emacs discussion 😁. Thanks for all of the time you are putting in!

@eddelbuettel
Copy link
Owner

There is a little bit of extra code in Rcpp (just type Rcpp:::RcppCxxFlags<RETURN> ) to protect us from Windows and other un$DEITY entities. I guess I skip the extra wrapping for Windows. That is a potential breaking point. :-/

@eddelbuettel
Copy link
Owner

Actually this should do the same and not call out to anything but R base:

      rcppdir <- system.file("include", package="Rcpp")
      if (.Platform$OS.type == "windows") rcppdir <- utils::shortPathName(normalizePath(rcppdir))
      rcppinclude <- paste("-I", rcppdir, sep="")
      cxxargs <- c(rcppinclude, cxxargs)	# prepend information from Rcpp

@jranke
Copy link
Contributor Author

jranke commented Nov 27, 2020

FWIW it looks good to me. But definitely out of my area of expertise.

@eddelbuettel eddelbuettel changed the title CRAN Check NOTES Multiple minor CRAN chec-related updates Nov 27, 2020
@jranke jranke changed the title Multiple minor CRAN chec-related updates Multiple minor CRAN check-related updates Nov 27, 2020
@eddelbuettel
Copy link
Owner

I now see

✔  checking examples (2.1s)
   Examples with CPU (user + system) or elapsed time > 5s
              user system elapsed
   cfunction 4.231  4.083   1.373
✔  checking for unstated dependencies in ‘tests’ ...

but I will deal with that (likely removing some examples now in tests) when add tests for cxxfunction which is the next TODO. After that, CRAN. Give a day or two or three...

@jranke
Copy link
Contributor Author

jranke commented Nov 27, 2020

OK, cool, let me know if there is anything else I could do. I will go back to adapt mkin accordingly now.

@eddelbuettel eddelbuettel merged commit cf30c01 into eddelbuettel:master Nov 27, 2020
@jranke jranke deleted the CRAN_checks branch November 27, 2020 19:56
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.

None yet

2 participants