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

chmod 600 to config file #50

Merged
merged 5 commits into from Nov 2, 2017

Conversation

@csgillespie
Copy link
Contributor

commented Nov 2, 2017

No description provided.

@eddelbuettel

This comment has been minimized.

Copy link
Owner

commented Nov 2, 2017

Good catch. Not sure why this now failed on Travis CI when it didn't before -- can you poke around?

csgillespie added some commits Nov 2, 2017

@csgillespie

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2017

In the second commit I commentted out my changes, i.e. the only difference with the master was a single line comment. It still failed. This implies a travis issue, rather than something to do with my commit.

In your test file you have

stopifnot(file.exists("~/.rpushbullet.json"))

Where/how is this file created?

For info, the test passes on my machine. But I have "~/.rpushbullet.json".

@eddelbuettel

This comment has been minimized.

Copy link
Owner

commented Nov 2, 2017

Where/how is this file created?

In .travis.yml:

rpushbullet/.travis.yml

Lines 19 to 22 in b66c07c

# decrypt the file .rpushbullet.json, cf https://docs.travis-ci.com/user/encrypting-files/
# condition this on have secure environment variables which PRs off forks do not
# cf https://docs.travis-ci.com/user/environment-variables/
- if [ "$TRAVIS_SECURE_ENV_VARS" == "true" ]; then openssl aes-256-cbc -K $encrypted_988d19a907a0_key -iv $encrypted_988d19a907a0_iv -in .rpushbullet.json.enc -out ~/.rpushbullet.json -d; fi

I think the stopifnot() is your enemy. Maybe just skip tests instead of aborting?

@eddelbuettel

This comment has been minimized.

Copy link
Owner

commented Nov 2, 2017

Also:

rpushbullet/.travis.yml

Lines 11 to 14 in b66c07c

# condition this on have secure environment variables which PRs off forks do not
# cf https://docs.travis-ci.com/user/environment-variables/
- if [ "$TRAVIS_SECURE_ENV_VARS" == "true" ]; then Run_RPushbullet_Tests="yes"; fi
- if [ "$TRAVIS_SECURE_ENV_VARS" == "true" ]; then Run_RPushbullet_Tests_All="yes"; fi

Seems like you were in fact set up to fail. Sorry about that.

But hey, I generally recommend to people to not open PRs without prior discussion :) [ Mostly for Rcpp et al though ... ]

csgillespie added some commits Nov 2, 2017

@csgillespie

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2017

I did know about your PR policy, but I figured a one line PR was just as easy for you to accept/reject as an issue followed by a PR.

I think there's an easy fix for travis

@eddelbuettel

This comment has been minimized.

Copy link
Owner

commented Nov 2, 2017

Do you mind if I just squash-merge it?

install:
- ./run.sh install_aptget r-cran-curl r-cran-jsonlite

script:
- ./run.sh run_tests
- if ["$TRAVIS_PULL_REQUEST" == "false"]; then ./run.sh run_tests; fi

This comment has been minimized.

Copy link
@eddelbuettel

eddelbuettel Nov 2, 2017

Owner

Hm. Now I will only ever get untested PRs. Not sure I like that for the longer term. Ok for now/today.

This comment has been minimized.

Copy link
@csgillespie

csgillespie Nov 2, 2017

Author Contributor

I think the "proper" solution would be for you to add the API key as an encrypted travis environment variable. Then create the json file on travis. A similar idea to the drat/travis stuff

This comment has been minimized.

Copy link
@eddelbuettel

eddelbuettel Nov 2, 2017

Owner

Could you take the lead on that, "copious spare time" permitting ?

@eddelbuettel eddelbuettel merged commit b566f6d into eddelbuettel:master Nov 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@csgillespie csgillespie referenced this pull request Nov 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.