Skip to content

Conversation

@stephenplusplus
Copy link
Contributor

@stephenplusplus stephenplusplus commented Apr 30, 2016

Fixes #1265

To Dos

  • Unit tests

// @callmehiphop

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 30, 2016
@stephenplusplus stephenplusplus force-pushed the spp--1265 branch 2 times, most recently from cc1a30d to dea768a Compare April 30, 2016 14:38
@stephenplusplus
Copy link
Contributor Author

@callmehiphop this is just another option, didn't mean for it to auto-invalidate your work. If you don't think this approach is the right way to go, just let me know! I'll wait before I get the tests ready.

@callmehiphop
Copy link
Contributor

It's cool, this was the first response I'd seen from you in regards to this issue in several days, so I figured you weren't a fan of what I was propsing. I originally did start going down this route, but I felt like the nesting/wrapping increased the cyclomatic complexity, which is why I decided to abandon it. If you think this is the best way to go, that's fine with me.

@stephenplusplus
Copy link
Contributor Author

It actually wasn't that I wasn't a fan, it was just hard to review from the 🚗 😢 👶 situation. Like you, I'm okay with all, but not a super-fan of any. I like this overall because it didn't force changes to retry-request (which I thought would be good originally, but in practice, turns out awkward [as you predicted]). I also like this because it doesn't duplicate any retry logic which already exists in retry-request. I'll get going on some tests.

@coveralls
Copy link

coveralls commented May 1, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 51da084 on stephenplusplus:spp--1265 into e9e4170 on GoogleCloudPlatform:master.

@callmehiphop callmehiphop merged commit eeafb33 into googleapis:master May 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants