[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-23 Thread nsuke
Github user nsuke commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-174161478 @szenker thanks for elaborating. I believe a minor ease of refactoring is not enough for silently ignored (non-)overrides. But if your plan of `peek` solves an exis

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread jeking3
Github user jeking3 commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173449026 I posted a C# runtime rewrite about 5 years ago that allowed for two way communication and multiple outstanding requests with out-of-order replies using the sequence num

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread ben-craig
Github user ben-craig commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173401300 @szenker It's probably not what you are looking for, but the concurrent client in C++ can allow multiple threads to have outstanding requests at once (though only one

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread hcorg
Github user hcorg commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173373046 @szenker pull requests are welcome :) right now I'm trying to revive my old branch (I'm porting some changes done to cpp lib into cpp_v2), currently it might not even comp

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread szenker
Github user szenker commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173338811 With the term "pipelining" I mean the following in this context: 1.) A client is allowed to send out a consecutive request even before it has received the response o

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread szenker
Github user szenker commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173337036 Adding the const keyword is not just syntactical sugar. It requires users which derive from TTransport to be more strict when they implement their own version of isOpen(

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread hcorg
Github user hcorg commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173236675 if it's a breaking change maybe we should put is as part of cpp_v2 developement? We could use override and other C++11 features to help users migrate. --- If your projec

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread jeking3
Github user jeking3 commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173236071 I'm a fan of const correctness. We shouldn't penalize const correctness due to an inherent weakness in the language (that was resolved in C++11, i.e. the [override oper

[GitHub] thrift pull request: THRIFT-3560: C++: declared TTransport::isOpen...

2016-01-20 Thread nsuke
Github user nsuke commented on the pull request: https://github.com/apache/thrift/pull/798#issuecomment-173229082 Implementation aside, this change breaks users' existing custom transports, although mostly in trivial ways. What is your (hypothetical) use case(s) with this or peek c