zerosnake0 commented on pull request #2181: URL: https://github.com/apache/thrift/pull/2181#issuecomment-722898457
> @zerosnake0 I replied some of your points below. I would also suggest you to read the associated [JIRA ticket](https://issues.apache.org/jira/browse/THRIFT-5233), which provides slightly more background about what problem this change solves. > > > 1. It's a breaking change > > Unlike Go1, there's no backward compatibility guarantee in thrift library. If you take a look at https://github.com/apache/thrift/blob/master/CHANGES.md, breaking changes are added in every release. > > > 1. the context argument is not really being used in the current implementation, the golang io interface does not use context neither ([golang/go#20280](https://github.com/golang/go/issues/20280)) > > It's used [here](https://github.com/apache/thrift/blob/05bb55148608b4324a8c91c21cf9a6a0dff282fa/lib/go/thrift/header_transport.go#L349-L361), [here](https://github.com/apache/thrift/blob/05bb55148608b4324a8c91c21cf9a6a0dff282fa/lib/go/thrift/compact_protocol.go#L338-L348). and [here](https://github.com/apache/thrift/blob/05bb55148608b4324a8c91c21cf9a6a0dff282fa/lib/go/thrift/binary_protocol.go#L468-L478). 1. do breaking change is only when necessary 2. I've already checked all usage of the context before I posted this, that's why i said "really". Furthermore, for the later two usage you wrote, i do not think that this kind of loop handling should be processed inside the protocol implementation ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
