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

Sourcing the exported extendr_wrappers.R #7

Merged
merged 5 commits into from
Jan 11, 2021

Conversation

CGMossa
Copy link
Member

@CGMossa CGMossa commented Nov 23, 2020

This makes use of pull-request #40 on extendr/extendr.

This test needs to be reworked for every setup..
@clauswilke
Copy link
Member

Doesn't this remove the need for all of this file: https://github.com/clauswilke/rextendr/blob/main/R/exported_functions.R

Also, with this PR we should make @hobofan a coauthor. His contribution is to remove 56 lines of code.

@clauswilke
Copy link
Member

And please don't add a test that cannot work because it looks for files only available on your hard drive. I had purposefully not added a test yet because it won't work until the various required crates are updated on crates.io. You can leave the code there but disable the test, or delete the test.

@CGMossa CGMossa marked this pull request as ready for review November 24, 2020 07:09
@clauswilke clauswilke mentioned this pull request Jan 11, 2021
@clauswilke
Copy link
Member

@CGMossa Could you merge in the current main branch and check that this PR still works?

@CGMossa
Copy link
Member Author

CGMossa commented Jan 11, 2021

Do you want me to press the squash and merge button or do you want me to pull from main and then push that pull onto this PR?

@clauswilke
Copy link
Member

I wanted you to do exactly what you just did. :-)

@clauswilke
Copy link
Member

Oh, but also try locally whether the resulting package still works. E.g., can you knit the vignette available here:
https://github.com/extendr/rextendr/blob/main/vignettes/rmarkdown.Rmd

@CGMossa
Copy link
Member Author

CGMossa commented Jan 11, 2021

Alright, I'm having some issues:

error[E0463]: can't find crate for `std`                       
  |
  = note: the `x86_64-pc-windows-gnu` target may not be installed

This got fixed after I changed rustup default stable-gnu; It was not only missing
target. It requires that the default rustup is not msvc, but gnu. I am mentioning this
as it was discussed that this will be passed to cargo directly.

Anyways, after this, the rmarkdown knits and everything works.

@clauswilke
Copy link
Member

Thanks!

@clauswilke clauswilke merged commit 455ed8a into extendr:main Jan 11, 2021
Ilia-Kosenkov pushed a commit to Ilia-Kosenkov/rextendr that referenced this pull request Jan 14, 2021
* Sourcing the exported extendr_wrappers.R

This makes use of extendr#40.

* Added a test

This test needs to be reworked for every setup..

* Removed wrapper function code.

Co-authored-by: hobofan <[email protected]>

* Skipping machine specific test

Co-authored-by: hobofan <[email protected]>
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.

2 participants