fishy commented on PR #2946: URL: https://github.com/apache/thrift/pull/2946#issuecomment-2000117594
The code looks good to me, but some suggestions: Can you please add a benchmark test with the example you gave in the jira ticket, and post the before/after result here, with `b.ReportAllocs()` called? since this benchmark test will require a thrift definition, you likely cannot have it under `lib/go/thrift` and will need it to be under `lib/go/test` instead. Depending on the result, I would also suggest you to write an unit test (also likely need to be under `lib/go/test`, in the same file as the benchmark test), that runs the benchmark test (via https://pkg.go.dev/testing#Benchmark), and checks the result, and verify that `AllocsPerOp` is below a certain threshold. -- 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. To unsubscribe, e-mail: notifications-unsubscr...@thrift.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
