Hi, Varun I think you don't need to split the PR into small ones, the current size is ok. But the PR description is too long to read, maybe you can simplify it. For the integration test, I have replied in the PR thread.
PS: I suggest you create discussions on github (https://github.com/apache/brpc/discussions), which is easier to track compared with emails. Thanks, Weibing Wang On Fri, May 29, 2026 at 11:27 AM Varun Raj <[email protected]> wrote: > > Hi all, > > Following up on Stage 1a (PR #3310). I realize the diff looks heavier > than the core change is: the source is ~900 lines, but the unit tests > plus the by-name mapping to upstream test vectors roughly double that, > which makes it a daunting review. I'd like to cut that load down. A > few options, happy to do any or all: > > 1. Split Stage 1a into three small, standalone PRs by module, in > dependency order: > - mysql_auth_packet (~220 lines) - length-encoded int/string and > packet-header helpers, no crypto. Reviewable in minutes and a good > first merge. > - mysql_auth_scramble (~320 lines) - the clean-room scramble/auth > crypto. This is the license-sensitive part flagged on #2093, so it > gets its own focused review. > - mysql_auth_handshake (~360 lines) - depends on packet. > > 2. Move the 75-test by-name upstream mapping table out of the PR > description into a short README under test/mysql_auth/, leaving just > summary counts in the PR. The table is provenance documentation, not > code, and right now it makes the PR read like a compliance audit. > > 3. Add an integration test that brings up a real MySQL server in CI > and exercises the handshake end to end against it. That gives far more > confidence in wire-correctness than the hand-written known-answer > vectors do, and it lets me trim the exhaustive unit vectors down to a > representative core. > > Is there one part that's the actual bottleneck for reviewers - test > volume, the clean-room provenance, or CI time - so I can prioritize? > And the earlier ask still stands: would a PMC member be able to assign > a Stage 1 shepherd from the rotation (wwbmmm the natural fit, but > anyone is welcome)? I'll do whatever lowers the load on reviewers. > > Thanks, > Varun Raj > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
