feat(protocol): add restrictApiVersion method
Nice, with a few extra commits to account for the updates that were missed, we're impressively getting a fully passing suite :)
Oops, my bad - missed those. Thanks for the help!
@dnwe shall I close https://github.com/IBM/sarama/pull/3158 and move forward with this one instead? If so I'll add a few tests to cover the changes
@trapped yeah, now it is in a passing state maybe squash the commits on this branch down into a single one and then have a quick think about whether this is the best structuring of this or how we could perhaps:
- decide if
requiredVersionis still necessary oncerestrictApiVersionexists - consider storing the "max-implemented" version numbers for each protocol api key in a single location and use those in both requiredVersion (if still required) and restrictApiVersion?
- consider whether the logic of restrictApiVersion itself could be centralised? Originally we added it as a func on each protocol type because that's the easiest way to allow it to modify the
Versionfield on the struct, but perhaps the func on each protocol could just be a shim to a generic shared func?
Hey @dnwe, sorry for the delay, couldn't find time for this until now.
After spending some more time learning the lib's structure I ended up addressing your suggestions and making things a bit neater in the process. To keep things clean I'm closing both the old PR and this one - I've pushed the new changes to https://github.com/IBM/sarama/pull/3209 🙂