szehon-ho commented on code in PR #57803: URL: https://github.com/apache/spark/pull/57803#discussion_r3731287876
########## AGENTS.md: ########## @@ -18,6 +18,8 @@ SQL golden file tests are managed by `SQLQueryTestSuite` and its variants. Read Spark Connect protocol is defined in proto files under `sql/connect/common/src/main/protobuf/`. Read the README there before modifying proto definitions. +When adding a member to an existing class, prefer a position that does not split up what is already there. A good default for a new private helper is after the public methods that use it, or alongside the file's existing helpers; in a test suite, after the `test(...)` blocks rather than between them. The common failure mode is placing a helper immediately above the first method that uses it, which interrupts a top-to-bottom read of the class's public surface or of the suite's list of cases. Conventions vary between files, so follow the local one rather than reorganizing. Review Comment: Good idea -- updated to a more general section-grouping principle: keep related kinds of code in sections and append a new member to the matching section, rather than a rule specific to private helpers. Kept a short note on the common failure mode of inserting a member next to its first use and splitting the surrounding section. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
