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

Feature/curl support #30

Merged
merged 5 commits into from Jan 26, 2017

Conversation

@restonslacker
Copy link
Contributor

@restonslacker restonslacker commented Jan 23, 2017

address issues in #29. principally removing the dependence on a system installation of the curl binary by using the cran package curl

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jan 23, 2017

The Travis break is "mine" as I prefer the old-school explicit Travis. Please add

  - ./travis-tool.sh install_aptget r-cran-curl

I will update the Travis file once we're done.

.travis.yml Outdated
@@ -18,6 +18,7 @@ before_install:

install:
- ./travis-tool.sh install_r RJSONIO
- ./travis-tool.sh install_r curl

This comment has been minimized.

@eddelbuettel

eddelbuettel Jan 23, 2017
Owner

If both go to install_r you can just have multiple args on one line.

But it is faster to do install_aptget r-cran-curl which should (!!) be resolvable. Else I address.

@eddelbuettel

This comment has been minimized.

Copy link

@eddelbuettel eddelbuettel commented on 0a63cde Jan 23, 2017

"request" makes me sound to powerful. More like "suggestion" ;-)

@restonslacker
Copy link
Contributor Author

@restonslacker restonslacker commented Jan 23, 2017

git commit --amend -m "change to install_aptget r-cran-curl per suggestion from @eddelbuettel"
=P

…warning is thrown.

Also added tests to make sure certain pushes fail.
@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jan 25, 2017

last commit looks good

need to get an updated digest package out the door, this may be next. getting to the point where I'd need a unified GH issue dashboard :-/

@restonslacker
Copy link
Contributor Author

@restonslacker restonslacker commented Jan 25, 2017

thanks!

there is one, but maybe it's a little too basic for you: https://github.com/issues

@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jan 25, 2017

yes, I should probably use that more.

Copy link
Owner

@eddelbuettel eddelbuettel left a comment

I will merge this and give it a spin.

@eddelbuettel eddelbuettel merged commit 3d8222a into eddelbuettel:master Jan 26, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@eddelbuettel
Copy link
Owner

@eddelbuettel eddelbuettel commented Jan 26, 2017

I made one post-PR commit to update a few things, now R CMD check is again happy.

You may need to pull this, and if we want to keep the other PR alive it may need some aligning.

@restonslacker
Copy link
Contributor Author

@restonslacker restonslacker commented Jan 27, 2017

WRT the changeLog, support for "address" pushes was removed as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.