From 31fedc2bc871d10bbc8e57a0c57e9006924e1bea Mon Sep 17 00:00:00 2001 From: Szehon Ho Date: Wed, 5 Aug 2026 17:39:16 -0700 Subject: [PATCH 1/2] [MINOR][DOC] Suggest where to place new class members in AGENTS.md Generated-by: Cursor (Opus 5) --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 36f1eccb02dd0..c9913d0cd0b11 100644 --- a/AGENTS.md +++ b/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. + Avoid introducing non-ASCII characters in code or comments. String literals may contain non-ASCII when the content requires it (error messages, test data, etc.). Identifiers are ASCII by convention. The common failure mode is typographic characters (em-dash, smart quotes, ellipsis, non-breaking space) sneaking into comments; scalastyle flags some of these. Spot-check before committing: `grep -rn -P "[^\x00-\x7F]" `. Keep source lines within 100 characters — the linters enforce this for Scala, Java, and Python, and LLMs commonly overrun it in comments and long expressions. A quick scan of just the changed files catches most cases in seconds, far cheaper than a CI round trip: From 9264b0793161b611e83a57dba3e948b7a1fb7788 Mon Sep 17 00:00:00 2001 From: Szehon Ho Date: Thu, 6 Aug 2026 12:01:06 -0700 Subject: [PATCH 2/2] Generalize the note into a section-based code placement principle Address review feedback: state the general principle of grouping members into sections and appending to the matching one, instead of a rule specific to private helpers. --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index c9913d0cd0b11..9a24c31c5b9bd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,7 @@ 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. +When adding members to an existing class or object, keep related kinds of code in sections rather than inserting into the middle of another section. A common layout is fields and constructors first, then the public/override API, then private helpers; in a test suite, tests together and helpers after them. Prefer appending to the matching section, or following the file's existing sectioning -- the common failure mode is inserting a new member next to its first use and splitting the surrounding section. Do not reorganize existing members unless the change requires it. Avoid introducing non-ASCII characters in code or comments. String literals may contain non-ASCII when the content requires it (error messages, test data, etc.). Identifiers are ASCII by convention. The common failure mode is typographic characters (em-dash, smart quotes, ellipsis, non-breaking space) sneaking into comments; scalastyle flags some of these. Spot-check before committing: `grep -rn -P "[^\x00-\x7F]" `.