uptake / updraft

R package for building flexible workflows

Other

13 stars 12 forks source link

misc fixes to make the library CRAN-able #27

Closed jameslamb closed 5 years ago

jameslamb commented 5 years ago

Hello sir,

I hope you'll consider this PR to make updraft CRAN-able. To build the package tarball that could be submitted to CRAN, you can run:

R CMD BUILD . --keep-empty-dirs
R CMD CHECK updraft_0.1.0.tar.gz --as-cran

I get that there is still an open question about whether this works on Windows, but as of this PR that should be the only remaining blocker to pushing to CRAN.

jameslamb commented 5 years ago

Sorry, I will get this working soon.

jameslamb commented 5 years ago

😱 so I can replicate this behavior on local. Tests run fine when I do devtools::test(), but doing this causes failures

R CMD BUILD .
R CMD CHECK updraft_0.1.0.tar.gz --as-cran

jameslamb commented 5 years ago

I don't know why these tests are breaking now :(

I'll keep trying

jameslamb commented 5 years ago

I just kicked off a rebuild on master and it's still working, so I guess I broken something?

master build (passed) --> https://travis-ci.org/UptakeOpenSource/updraft/builds/501294059 my most recent PR build (failed) --> https://travis-ci.org/UptakeOpenSource/updraft/builds/479659617?utm_source=github_status&utm_medium=notification

pretty confused

jameslamb commented 5 years ago

I bet I need to put back the ::: in LogStackTrace()

jameslamb commented 5 years ago

I'm so confused. I cannot reproduce this issue on my laptop.

This runs fine:

R CMD BUILD .
R CMD CHECK *.tar.gz --as-cran

0_o

codecov-io commented 5 years ago

Codecov Report

Merging #27 into master will decrease coverage by 0.15%. The diff coverage is 20%.

@@            Coverage Diff             @@
##           master      #27      +/-   ##
==========================================
- Coverage   82.88%   82.72%   -0.16%     
==========================================
  Files          14       14              
  Lines         625      625              
==========================================
- Hits          518      517       -1     
- Misses        107      108       +1

Impacted Files	Coverage Δ
R/utils_modules.R	`49.09% <20%> (ø)`	:arrow_up:
R/workflow_dag.R	`91.62% <0%> (-0.47%)`	:arrow_down:

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 e3cfa59...4775948. Read the comment docs.

jameslamb commented 5 years ago

Hey @cwschultz88 it worked!!

I had to change your unit tests. Basically you were writing output files to the same directory as the tests and that was running into some weird file permissions errors. Once I changed to creating a random output directory with tempdir(), that issue went away.

Please review and consider this PR to get updraft closer to CRAN-ready.

jameslamb commented 5 years ago

@cwschultz88 I think this can be merged

jameslamb commented 5 years ago

@mjermann could I get a review on this? I think once we fix it, it will also fix the issue Shreyas is seeing on #31