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.

Sign up for GitHub

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

Jump to bottom

Adding context version of Check and Response.Apply methods. #8

Merged
merged 6 commits into from
Sep 25, 2018
Merged

Adding context version of Check and Response.Apply methods. #8

merged 6 commits into from
Sep 25, 2018

Conversation

henvic
Copy link
Contributor

@henvic henvic commented Nov 1, 2017

Supersedes #7.

@zenorocha
Copy link

Ping

@henvic
Copy link
Contributor Author

henvic commented Sep 17, 2018

Hi @kyleconroy, I see you have done some commits on this project recently. Can you please take a look at this?

I believe it's pretty straight-forward and shouldn't take much time.

Thanks.

@kyleconroy
Copy link
Contributor

@henvic yes, can do. I honestly had forgotten this was open. Sorry!

kyleconroy
kyleconroy suggested changes Sep 20, 2018
View reviewed changes
Copy link
Contributor

@kyleconroy kyleconroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small changes. Sorry again that this review took so long to happen.

sdk_ctx.go Outdated Show resolved Hide resolved
sdk_ctx.go Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
kyleconroy
kyleconroy suggested changes Sep 21, 2018
View reviewed changes
.travis.yml Outdated Show resolved Hide resolved
@kyleconroy
Copy link
Contributor

There appears to be a go vet failure in sdk_test.go which is unrelated to your changes. Either you'll need to fix it or you'll need to remove 1.10 and 1.11 from the matrix and I can fix the issue in a separate PR.

@kyleconroy kyleconroy merged commit 5205c98 into equinox-io:master Sep 25, 2018
@kyleconroy
Copy link
Contributor

Thanks @henvic. I'll cut a new release that includes the new methods

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

@kyleconroy kyleconroy kyleconroy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@henvic @zenorocha @kyleconroy