Skip to content

Conversation

@dapengzhang0
Copy link
Contributor

Resolves #3684

@dapengzhang0 dapengzhang0 requested a review from ejona86 November 14, 2017 01:35
@dapengzhang0 dapengzhang0 added the TODO:backport PR needs to be backported. Removed after backport complete label Nov 14, 2017
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

Is it easy to add a test?

@dapengzhang0
Copy link
Contributor Author

Unit test seems not straightforward. Considering the effort it would take, and that keepalive is still not fully test covered, maybe defining and implementing an interop test is better solution.

@ejona86
Copy link
Member

ejona86 commented Nov 14, 2017

@dapengzhang0, that's fine. We'll just not add a test. Interop would have problems with sleeping and flakiness.

@dapengzhang0 dapengzhang0 merged commit 66f9ef5 into grpc:master Nov 14, 2017
@dapengzhang0 dapengzhang0 removed the TODO:backport PR needs to be backported. Removed after backport complete label Nov 14, 2017
@carl-mastrangelo carl-mastrangelo added this to the 1.9 milestone Nov 30, 2017
@dapengzhang0 dapengzhang0 deleted the fix3684 branch February 26, 2018 19:05
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants