yuqi1129 commented on code in PR #11339: URL: https://github.com/apache/gravitino/pull/11339#discussion_r3354248190
########## AGENTS.md: ########## @@ -42,10 +43,64 @@ 4. Constructors. 5. Methods (Group by visibility, putting `private` methods at the end). +## Code Design & Structure +- **Method granularity**: Each method must earn its existence. Avoid small, fragmented single-use helpers — keep cohesive logic together. At the same time, fold genuinely duplicated logic into one shared helper instead of repeating it. Extract when it is reused or names a genuinely non-obvious step; inline trivial one-liners. +- **No dead parameters or methods**: Remove a parameter that is always the same constant (for example, a flag that is always `true`) and a method that does nothing meaningful. Keep related declarations and their setup together (for example, a thread-pool field and the loop that submits tasks to it). +- **Field assignment**: Qualify field assignments with `this.` (for example, `this.workers = ...`) in constructors and setters. +- **Test-only code must not live in production classes**: Never add a production method or field that exists only to serve a test. + - Prefer keeping test-only logic in the test module (test helpers, fixtures, or the test class itself). + - If a production member genuinely must be reachable from a test, annotate it with `@VisibleForTesting` and keep the narrowest visibility possible (package-private, not `public`/`protected`). Never widen visibility solely for a test. + - Do not reach into non-public members from tests via reflection (`setAccessible(true)`). Add a `@VisibleForTesting` constructor/factory instead. Review Comment: I see. -- 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]
