diff --git a/GEMINI.md b/GEMINI.md index 21452271b..f3186f7f3 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -1,6 +1,6 @@ # Engineering Standards for Gemini CLI -This document outlines foundational mandates, architectural patterns, and project-specific conventions to ensure high-quality, idiomatic, and consistent code from the first iteration. +This document outlines foundational mandates, architectural patterns, and project-specific conventions to ensure high-quality, idiomatic, and consistent code from the first iteration. When modifying this file, always review the full document to prevent the introduction of duplicate instructions and ensure the content remains coherent and logically organized. ## Core Mandates @@ -8,7 +8,7 @@ This document outlines foundational mandates, architectural patterns, and projec - **Addition:** When adding new symbols, ensure the corresponding import is added. - **Removal:** When removing the last usage of a class or symbol from a file (e.g., removing a `@Inject Clock clock;` field), **immediately remove the associated import**. Do not wait for a build failure to identify unused imports. - **No Redundant Qualifications:** NEVER use fully qualified class names (e.g., `java.time.temporal.ChronoUnit.DAYS`) in code when an import can be used instead. Always prefer adding an import and using the simple name. -- **Static Imports for DateTimeUtils:** Always statically import `toInstant` and `toDateTime` from `google.registry.util.DateTimeUtils`. NEVER write them as `DateTimeUtils.toInstant(...)` or `google.registry.util.DateTimeUtils.toInstant(...)` at the call site; use `toInstant(...)` and `toDateTime(...)` directly. +- **Static Imports for Utilities:** Always statically import methods from utility classes like `DateTimeUtils` or `CacheUtils`. (e.g., use `toInstant(...)` instead of `DateTimeUtils.toInstant(...)`). - **Checkstyle:** Proactively fix common checkstyle errors (line length > 100, formatting, unused imports) during the initial code write. Do not wait for CI/build failures to address these, as iterative fixes are inefficient. - **Verification**: Before finalizing any change, scan the imports section for redundancy. - **License Headers**: When creating new files, ensure the license header uses the current year (e.g., 2026). Existing files should retain their original year. @@ -21,6 +21,7 @@ This document outlines foundational mandates, architectural patterns, and projec - Avoid direct calls to `Instant.now()`, `DateTime.now()`, `ZonedDateTime.now()`, or `System.currentTimeMillis()`. - Inject `google.registry.util.Clock` (production) or `google.registry.testing.FakeClock` (tests). - Use `clock.nowDate()` to get a `ZonedDateTime` in UTC. + - When defining timestamps for tests, prefer using a fixed, static constant (e.g., `Instant.parse("2024-03-27T10:15:30.105Z")`) over capturing `clock.now()` to prevent flaky tests caused by the passage of real time. Avoid using the Unix epoch (`START_INSTANT`) unless specifically testing epoch-related logic; instead, use realistic dates and vary them across different test suites to ensure logic isn't dependent on a specific "standard" date. - **Beam Pipelines:** - Ensure `Clock` is serializable (it is by default in this project) when used in Beam `DoFn`s. - Pass the `Clock` through the constructor or via Dagger provider methods in the pipeline module. @@ -56,6 +57,19 @@ This document outlines foundational mandates, architectural patterns, and projec - **Turn Minimization:** Aim for "perfect" code in the first iteration. Iterative fixes for checkstyle or compilation errors consume significant context and time. - **Context Management:** Use sub-agents for batch refactoring or high-volume output tasks to keep the main session history lean and efficient. +## General Code Review Lessons & Avoidable Mistakes +Based on historical PR reviews, avoid the following common mistakes: +- **No Unnecessary Casts:** Do not unnecessarily cast objects if the method signature accepts the type directly (e.g., avoid `(Instant) fakeClock.now()` or `(ImmutableSet) bsaQuery(...)` if it compiles without it). +- **Visibility Modifiers:** Do not use `/* package */` comments to denote package-private visibility. Just leave the modifier blank; it is an established idiom in this codebase. + +### Advanced Java & Guava Idioms +- **Immutable Types:** Declare variables, fields, and return types explicitly as Guava immutable types (e.g., `ImmutableList`, `ImmutableMap`) instead of their generic interfaces (`List`, `Map`) to clearly communicate immutability contracts to callers. Use `toImmutableList()` and `toImmutableMap()` collectors in streams rather than manually accumulating into an `ArrayList` or `HashMap`. +- **Constructors:** Do not perform heavy logic, I/O, or external API calls inside constructors. Initialization logic should be deferred or handled in a factory method or a dedicated startup routine. +- **Exception Handling:** Do not catch generic `Exception` or `Throwable` if a more specific exception is expected. Never "log and re-throw" the same exception; either handle it entirely (and log), or throw it up the chain. For batch processes, catch exceptions at the individual item/chunk level so one failure doesn't abort the entire batch. +- **Fail Fast:** Validate inputs and fail fast (using `Preconditions.checkArgument` or similar) at the highest level possible rather than passing invalid state (like `null`s) deeper into business logic. +- **Magic Numbers:** Always document magic numbers or hardcoded limits (like `50.0` or `30`) with inline comments explaining the rationale. +- **Null Safety and Optional:** Prefer using `Optional` for any variable that is expected to potentially be null. For any other variable that can be null but cannot use an `Optional` (e.g., function parameters or return types where `Optional` is not idiomatic), it MUST be annotated with `@Nullable`. Always use the `javax.annotation.Nullable` annotation. + --- # Gemini Engineering Guide: Nomulus Codebase @@ -72,6 +86,7 @@ This document captures high-level architectural patterns, lessons learned from l - Year Arithmetic: Use `DateTimeUtils.plusYears()` and `DateTimeUtils.minusYears()` to handle February 29th logic correctly. ## Source Control +- **Committing:** Always create a new commit on the branch if one hasn't been created yet for the branch's specific work. Only perform amending (`git commit --amend --no-edit`) for subsequent changes once the initial commit has been successfully created. - **One Commit Per PR:** All changes for a single PR must be squashed into a single commit before merging. - **Default to Amend:** Once an initial commit is created for a PR, all subsequent functional changes should be amended into that same commit by default (`git commit --amend --no-edit`). This ensures the PR remains a single, clean unit of work throughout the development lifecycle. - **Commit Message Style:** Follow standard Git commit best practices. The subject line (first line) should be concise, capitalized, and **must not end with punctuation** (e.g., a period). @@ -122,4 +137,3 @@ This protocol defines the standard for interacting with GitHub repositories and ## 3. PR Lifecycle Management - **One Commit Per PR:** Ensure all changes are squashed into a single, clean commit. Use `git commit --amend --no-edit` for follow-up fixes. - **Clean Workspace:** Always run `git status` and verify the repository state before declaring a task complete. -