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

fix for token service account #313

Merged
merged 4 commits into from Jan 5, 2016

Conversation

Projects
None yet
2 participants
@nathangoulding
Contributor

nathangoulding commented Jan 5, 2016

@hadley This fixes the root cause of the Error: is.request(y) is not TRUE that you've been seeing.

It was only breaking when using the service token, and is caused by the sign method not returning what request_combine is expecting. PTAL.

nathangoulding added some commits Dec 21, 2015

@hadley

This comment has been minimized.

Member

hadley commented Jan 5, 2016

This looks good! Two requests:

  • Could you please add a bullet point to news?
  • Could you add an explicit stopifnot(is.request(x)) in the right place to make sure we don't accidentally introduce this problem in the future? (i.e. if we add more token classes)
@nathangoulding

This comment has been minimized.

Contributor

nathangoulding commented Jan 5, 2016

Great, I think I've addressed these.

hadley added a commit that referenced this pull request Jan 5, 2016

Merge pull request #313 from nathangoulding/master
fix for token service account

@hadley hadley merged commit adfe696 into r-lib:master Jan 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hadley

This comment has been minimized.

Member

hadley commented Jan 5, 2016

Thanks!

For future reference, it's a bit better to put news items at the top - it seems to generate fewer spurious merge conflicts.

@nathangoulding

This comment has been minimized.

Contributor

nathangoulding commented Jan 5, 2016

Ahh OK, roger that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment