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

subset animals on windows for when by='random' #40

Merged
merged 2 commits into from Oct 15, 2015

Conversation

ateucher
Copy link
Contributor

Sorry to do this just after CRAN submission, but I didn't notice the change in dealing with unicode on windows until I installed from CRAN today.

When you set by = 'random' (on Windows), it passes the stop on line 8, but the full list of animals is still available to sample from, resulting in a bad unicode animal showing up once in a while.

This PR subsets the animals vector by removing the animals defined in ua

@sckott
Copy link
Owner

sckott commented Oct 14, 2015

Thanks, and I should have checked with you after that change!

@ateucher
Copy link
Contributor Author

No worries! I have one more commit coming with a test - don't merge quite yet ;)

@sckott
Copy link
Owner

sckott commented Oct 14, 2015

cool

@ateucher
Copy link
Contributor Author

I think using skip_on_os(c("mac", "linux", "solaris")) should make sure it only runs on Windows (i.e., not Travis)

@ateucher
Copy link
Contributor Author

Except apparently AppVeyor isn't installing an up-to-date testthat with the skip_on_os function. Was testthat so recently updated?

@sckott
Copy link
Owner

sckott commented Oct 14, 2015

yeah, testthat was updated today i think https://cran.rstudio.com/web/packages/testthat/

@ateucher
Copy link
Contributor Author

Ha, what are the odds? I updated a bunch of packages this morning, testthat must have been in there...

@sckott
Copy link
Owner

sckott commented Oct 15, 2015

we can add install testthat from github after it's merged.

sckott added a commit that referenced this pull request Oct 15, 2015
subset animals on windows for when by='random'
@sckott sckott merged commit 942eef2 into sckott:master Oct 15, 2015
@sckott
Copy link
Owner

sckott commented Oct 15, 2015

looks like we're working now after using dev testthat

@ateucher
Copy link
Contributor Author

👍

@sckott sckott modified the milestone: v0.5 Dec 15, 2016
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