vtprotobuf icon indicating copy to clipboard operation
vtprotobuf copied to clipboard

Add support for pools for oneof fields

Open maticz opened this issue 3 years ago • 5 comments

I have implemented the minimal changes necessary for pools to work with oneof fields because the generated code currently does not compile. If needed, I can work on improving the code, add some tests and so on but I'd be interested first in knowing whether you'd be willing to merge something like this.

maticz avatar Sep 24 '21 14:09 maticz

Yeah for sure, sorry about that missing functionality, we just don't have any oneof fields in Vitess. If you can write some tests for this I'll definitely merge. :+1:

vmg avatar Sep 27 '21 13:09 vmg

@maticz Hello, we would like to make use of the pooling of oneof fields. Would you mind If I build on top of your work and add the tests in a separate PR?

coufalja avatar Jan 10 '22 08:01 coufalja

Hey @coufalja, sure, go ahead. I'd be glad if this functionality was merged.

maticz avatar Jan 10 '22 19:01 maticz

@vmg Hi! Sorry for the ping. Your lib is getting quite popular and we are starting to use it. This is a critical feature for us, as we also have oneofs. Could you consider merging this ASAP?

DoubleDi avatar Mar 05 '22 09:03 DoubleDi

@DoubleDi Hi! I'm afraid that this PR doesn't have tests, and since we don't use OneOf fields in Vitess, I cannot properly verify that the behavior is accurate. If you're interested in seeing this PR merged, writing tests for it would be a great first step. Thank you!

vmg avatar Mar 07 '22 09:03 vmg

@Fumesover has kindly written tests for the feature in https://github.com/planetscale/vtprotobuf/pull/83 -- this is now implemented.

vmg avatar Aug 29 '23 07:08 vmg