Skip to content

Feature/travis via docker#21

Merged
eddelbuettel merged 4 commits intomasterfrom
feature/travis_via_docker
Dec 16, 2018
Merged

Feature/travis via docker#21
eddelbuettel merged 4 commits intomasterfrom
feature/travis_via_docker

Conversation

@eddelbuettel
Copy link
Owner

Seems to work just fine

@eddelbuettel eddelbuettel mentioned this pull request Dec 16, 2018
Copy link
Collaborator

@aaronwolen aaronwolen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I'm really liking this approach.

Co-Authored-By: eddelbuettel <edd@debian.org>
@eddelbuettel
Copy link
Owner Author

Looks great! I'm really liking this approach.

Thank you! I was a little hesitant as I did not want to come across as too heavy-handed when just about everybody else uses 'default' Travis. But this does have clear advantages in my view: the portability of the Docker container widens the applicability, and as a side effect gives people the ability to deploy. (There are are downsides: if we needed new packages in the container we'd first have to commit; the cycle on that is short though as the Docker build service usually turns around within minutes.)

skip_on_cran

I totally missed that. I just used GitHub fancyness as committed your suggestion here in the GUI. I'm open -- if these are useful tests, should they not run at CRAN too? Why skip?

@eddelbuettel eddelbuettel merged commit d11f922 into master Dec 16, 2018
@eddelbuettel eddelbuettel deleted the feature/travis_via_docker branch December 16, 2018 19:10
@aaronwolen
Copy link
Collaborator

I was concerned these tests might put an unnecessary burden on CRAN's build servers, especially as the number of tests start to accumulate. What do you think?

@eddelbuettel
Copy link
Owner Author

Ah, I see -- good thinking.

But there is a protocol for this: we generally have 'one minute' (which is not too bad) and check logs report that. So I'd say for now we can turn them on. Given that you wrote them :)

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