qzyu999 commented on code in PR #2922: URL: https://github.com/apache/fluss/pull/2922#discussion_r2996314183
########## AGENTS.md: ########## @@ -0,0 +1,441 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +# Apache Fluss - AI Agent Coding Guide + +AI coding guide for Apache Fluss with critical rules, patterns, and standards derived from codebase analysis and Checkstyle enforcement. + +**Purpose:** This guide helps AI coding agents contribute to Apache Fluss by providing project-specific conventions, architectural patterns, and quality standards. It covers both code contribution (Sections 1-10) and deployment/setup guidance (Section 11). + +**Sections:** 1. Critical Rules | 2. API Patterns | 3. Code Organization | 4. Error Handling | 5. Concurrency | 6. Testing | 7. Dependencies | 8. Configuration | 9. Serialization/RPC | 10. Module Boundaries | 11. Build & CI | 12. Git & Pull Requests | 13. AI Agent Boundaries + +--- + +## 1. Critical Rules (MUST/NEVER) + +**Enforced by Checkstyle** - violations will fail CI. + +### Dependencies & Utilities + +**FORBIDDEN imports** (use shaded versions - see Section 7): +```java +import com.google.common.* // → org.apache.fluss.shaded.guava.* +import com.fasterxml.jackson.*, org.codehaus.jackson.* // → org.apache.fluss.shaded.jackson2.* +import io.netty.* // → org.apache.fluss.shaded.netty4.* +import org.apache.arrow.* // → org.apache.fluss.shaded.arrow.* +import org.apache.zookeeper.* // → org.apache.fluss.shaded.zookeeper38.* +``` + +**MANDATORY utility substitutions:** +```java +// ❌ new ConcurrentHashMap<>() → ✅ MapUtils.newConcurrentMap() (see https://github.com/apache/fluss/issues/375) +// ❌ com.google.common.base.Preconditions → ✅ org.apache.fluss.utils.Preconditions (import statically) +// ❌ com.google.common.annotations.VisibleForTesting → ✅ org.apache.fluss.annotation.VisibleForTesting +// ❌ org.apache.commons.lang3.SerializationUtils → ✅ org.apache.fluss.utils.InstantiationUtil +// ❌ Boolean.getBoolean("prop") → ✅ Boolean.parseBoolean(System.getProperty("prop")) +``` + +### Testing + +**MANDATORY: Use AssertJ, NOT JUnit assertions:** +```java +// ❌ Assertions.assertEquals(expected, actual) +// ✅ assertThat(actual).isEqualTo(expected) + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +assertThat(list).hasSize(3).contains("a", "b"); +assertThatThrownBy(() -> doSomething()).isInstanceOf(IllegalArgumentException.class); +``` + +**NEVER use @Timeout on tests** - rely on global timeout + +### Code Style + +- **NEVER use star imports** (`import java.util.*;`) - set IDE threshold to 9999 +- **NEVER have trailing whitespace** - run `./mvnw spotless:apply` +- **ALWAYS use Java-style arrays:** `String[] args` NOT `String args[]` +- **ALWAYS require braces:** `if (x) { doIt(); }` NOT `if (x) doIt();` +- **NEVER use TODO(username)** - use `TODO:` without username +- **NEVER use FIXME, XXX, or @author tags** - use git history + +### Documentation + +- **File size limit:** 3000 lines max +- **Javadoc REQUIRED:** All protected/public classes, interfaces, enums, methods + +--- + +## 2. API Design Patterns + +### API Stability Annotations + +```java +@PublicStable // Stable - breaking changes only in major versions +@PublicEvolving // May change in minor versions +@Internal // Not public API - can change anytime +``` + +**Usage:** `@PublicStable` for core APIs (`Connection`, `Admin`); `@PublicEvolving` for new features; `@Internal` for RPC/internals +**Reference:** `fluss-common/src/main/java/org/apache/fluss/annotation/` + +### Builder Pattern + +```java +ConfigOption<Duration> timeout = ConfigBuilder + .key("client.timeout") + .durationType() + .defaultValue(Duration.ofSeconds(30)); +``` + +**Pattern:** Static inner `Builder` class, method chaining, private constructor, `build()` method +**Reference:** `fluss-common/src/main/java/org/apache/fluss/config/ConfigBuilder.java` + +### Factory Pattern + +```java +public class ConnectionFactory { + private ConnectionFactory() {} // Private constructor + + public static Connection createConnection(Configuration conf) { + return new FlussConnection(conf); + } +} +``` + +**Rules:** Private constructor, static factory methods, return interface types +**Reference:** `fluss-client/src/main/java/org/apache/fluss/client/ConnectionFactory.java` + +### Additional Patterns + +- **Interface Segregation:** Provide generic (`Lookuper`) and typed (`TypedLookuper<T>`) variants +- **Result Objects:** Immutable `final` classes with `private final` fields, no setters, implement `equals()`/`hashCode()`/`toString()` +- **Thread Safety:** Document with `@ThreadSafe` or `@NotThreadSafe` annotations + +--- + +## 3. Code Organization + +### Naming Conventions + +| Type | Convention | Example | +|------|-----------|---------| +| **Interface** | Plain descriptive name | `Connection`, `Admin`, `LogScanner` | +| **Implementation** | Suffix `Impl` or descriptive name | `AdminImpl`, `NettyClient`, `RocksDBKvStore` | +| **Abstract class** | Prefix `Abstract` | `AbstractIterator`, `AbstractGoal` | +| **Utility class** | Suffix `Utils` (private constructor, static methods) | `MapUtils`, `StringUtils`, `IOUtils` | +| **Test class** | Suffix `Test` (unit) or `ITCase` (integration) | `ConfigBuilderTest`, `ServerITCaseBase` | +| **Test utility** | Prefix `Testing` | `TestingRemoteLogStorage` | +| **Exception** | Suffix `Exception` | `TableNotExistException` | + +### Package Structure + +See CLAUDE.md for full module/package organization. Key modules: `fluss-common`, `fluss-rpc`, `fluss-client`, `fluss-server`. Review Comment: I don't see a `CLAUDE.md` in the repo, actually if you check with Airflow they do have a `CLAUDE.md`, but it simply points to `AGENTS.md` in its contents: https://github.com/apache/airflow/blob/main/CLAUDE.md. I am thinking we could do the same. -- 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]
