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

[my_unzip] add exception #1678

Merged
merged 2 commits into from Dec 22, 2017

Conversation

Projects
None yet
2 participants
@theGreatWhiteShark
Contributor

theGreatWhiteShark commented Dec 22, 2017

Whenever R is compiled with no 'unzip' package installed in the underlying operation system, the getOption( 'unzip' ) variable is set to an empty string. (At least for R-3.4.0 on a vanilla Ubuntu16.04

Since the empty string doesn't trigger the if clause and its usage of the R interal utils::unzip function, I added another exception.

[my_unzip] add exception
Whenever R is compiled with no 'unzip' package installed in the underlying operation system, the getOption( 'unzip' ) variable is set to an empty string. (At least for R-3.4.0 on a vanilla Ubuntu16.04

Since the empty string doesn't trigger the if clause and its usage of the R interal utils::unzip function, I added another exception.
@jimhester

This comment has been minimized.

Member

jimhester commented Dec 22, 2017

Thanks! This is fine, you just need to use the non-vectorized || function.

Make that change and add a note to NEWS.md mentioning this change, the PR number and your GitHub handle and I can merge this.

[my_unzip] add exception +
In addition to the previous commit a non-vectorized 'or' is used within the my_unzip function.

The changes have been added to the NEWS.md
@theGreatWhiteShark

This comment has been minimized.

Contributor

theGreatWhiteShark commented Dec 22, 2017

Alright, done.

@jimhester

This comment has been minimized.

Member

jimhester commented Dec 22, 2017

Thanks!

@jimhester jimhester merged commit 0bcfd6e into r-lib:master Dec 22, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment