You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by lh...@apache.org on 2026/05/27 19:14:57 UTC

(pulsar) branch lh-AGENTS.md created (now e8c836a118d)

This is an automated email from the ASF dual-hosted git repository.

lhotari pushed a change to branch lh-AGENTS.md
in repository https://gitbox.apache.org/repos/asf/pulsar.git


      at e8c836a118d [improve][misc] Add do/don't code examples for key conventions in CODING.md

This branch includes the following new commits:

     new e8c836a118d [improve][misc] Add do/don't code examples for key conventions in CODING.md

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(pulsar) 01/01: [improve][misc] Add do/don't code examples for key conventions in CODING.md

Posted by lh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lhotari pushed a commit to branch lh-AGENTS.md
in repository https://gitbox.apache.org/repos/asf/pulsar.git

commit e8c836a118dc4e2ba999fd7e25113fe82757ba3d
Author: Lari Hotari <lh...@apache.org>
AuthorDate: Wed May 27 22:14:35 2026 +0300

    [improve][misc] Add do/don't code examples for key conventions in CODING.md
    
    Address PR review feedback: AI agents follow concrete patterns more
    reliably than prose. Add short Avoid/Prefer code examples (matching the
    labeled-block style of the former .github/copilot-instructions.md) for:
    CompletableFuture must not throw synchronously, flatten nested futures
    with thenCompose, prefer a record over Pair, and record map keys over
    concatenated strings. Examples use generic placeholder names so they
    read as illustrations, not real Pulsar APIs.
---
 CODING.md | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/CODING.md b/CODING.md
index 230e03b9a0d..edff7e5e02f 100644
--- a/CODING.md
+++ b/CODING.md
@@ -37,10 +37,44 @@ Pulsar relies heavily on `CompletableFuture`; prefer it over `ListenableFuture`
   the returned future — `return CompletableFuture.failedFuture(e);` — including for argument validation
   (`if (arg == null) return CompletableFuture.failedFuture(new IllegalArgumentException("arg"));`).
   Throwing *inside* a stage (`thenApply`, `thenCompose`, `handle`, `whenComplete`, …) is fine.
+
+  Avoid (escapes synchronously; a caller chaining `.exceptionally(...)` never sees it):
+
+  ```java
+  CompletableFuture<T> process(String arg) {
+      if (arg == null) {
+          throw new IllegalArgumentException("arg");
+      }
+      return doProcessAsync(arg);
+  }
+  ```
+
+  Prefer (report the validation failure through the returned future):
+
+  ```java
+  CompletableFuture<T> process(String arg) {
+      if (arg == null) {
+          return CompletableFuture.failedFuture(new IllegalArgumentException("arg"));
+      }
+      return doProcessAsync(arg);
+  }
+  ```
 - **Never block on event-loop / async-execution threads** — no `Thread.sleep`, `Future.get()`,
   `CompletableFuture.join()`, or blocking IO. An operation that performs IO should return a future.
 - **Avoid nested futures** (`CompletableFuture<CompletableFuture<T>>`); flatten with `thenCompose`.
   Prefer **`OrderedExecutor`** for ordered asynchronous work.
+
+  Avoid (`thenApply` on a future-returning function yields `CompletableFuture<CompletableFuture<R>>`):
+
+  ```java
+  return firstAsync(arg).thenApply(v -> secondAsync(v));
+  ```
+
+  Prefer (`thenCompose` flattens it to `CompletableFuture<R>`):
+
+  ```java
+  return firstAsync(arg).thenCompose(v -> secondAsync(v));
+  ```
 - **Converting a synchronous-throwing method to a failed future is not mechanical** — some callers rely
   on the throw happening *before* the async work starts, so evaluate each call site. Use a shared
   `checkArgumentAsync` helper (in `FutureUtil`) to validate without duplicating try/catch.
@@ -94,9 +128,37 @@ but much existing code isn't, so integration-style is the pragmatic default. See
 - **Don't return generic tuples.** Instead of `org.apache.commons.lang3.tuple.Pair<L, R>` (or a similar
   tuple type), define a small, purpose-named **Java `record`** inline in the class that declares the
   method, with the **same visibility as the method** (`public`, package-private, or `private`).
+
+  Avoid (positional and untyped; call sites read `getLeft()` / `getRight()`):
+
+  ```java
+  private Pair<Integer, Integer> minMax(Collection<Integer> values) { ... }
+  ```
+
+  Prefer (a purpose-named record with the same visibility as the method):
+
+  ```java
+  private record MinMax(int min, int max) {}
+  private MinMax minMax(Collection<Integer> values) { ... }
+  ```
 - **Prefer record keys over concatenated strings.** For a composite `Map` key, use a small `record`
   instead of concatenating a `String` (e.g. `a + ":" + b`) — correct `equals`/`hashCode`, type-safe,
   no delimiter/escaping bugs.
+
+  Avoid (delimiter collisions when a value contains `:`; no type safety):
+
+  ```java
+  Map<String, V> map = new HashMap<>();
+  map.get(a + ":" + b);
+  ```
+
+  Prefer (a small record key with correct `equals`/`hashCode`):
+
+  ```java
+  record Key(String a, String b) {}
+  Map<Key, V> map = new HashMap<>();
+  map.get(new Key(a, b));
+  ```
 - **Don't use `@Builder` on public client-API classes** (harder to maintain backwards compatibility) — hand-write the builder.
 - **Name methods for intent.** A method's name should reveal what it does. Query methods read like
   queries (`shouldSkipChunk`, not `skipChunk`); methods that mutate state or perform an action are