This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new f546f7f0702 Clarify when code comments preserve useful context (#73359)
f546f7f0702 is described below
commit f546f7f070230058e6956f2f51d0196b5da8f85e
Author: Shahar Epstein <[email protected]>
AuthorDate: Sun Sep 20 11:53:16 2026 +0300
Clarify when code comments preserve useful context (#73359)
---
.github/instructions/code-review.instructions.md | 8 +++++++-
AGENTS.md | 9 ++++++++-
dev/skill-evals/last-eval-hash.txt | 2 +-
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/.github/instructions/code-review.instructions.md
b/.github/instructions/code-review.instructions.md
index da2674c4c1a..b047c576e5e 100644
--- a/.github/instructions/code-review.instructions.md
+++ b/.github/instructions/code-review.instructions.md
@@ -65,7 +65,13 @@ Flag these patterns that indicate low-quality AI-generated
contributions:
- **Description doesn't match code**: PR description describes something
different from what the code actually does.
- **No evidence of testing**: Claims of fixes without test evidence, or author
admitting they cannot run the test suite.
- **Over-engineered solutions**: Adding caching layers, complex locking, or
benchmark scripts for problems that don't exist or are misunderstood.
-- **Narrating or redundant comments**: Comments that restate what the next
line does (e.g., `# Add the item to the list` before `list.append(item)`);
multi-line prose explaining a one-line change; the same rationale repeated at
several sites; or explanatory comments on tests whose names already convey
intent. Comments should explain *why* when it is non-obvious, not narrate
*what*. Flag over-commenting as noise to be trimmed.
+- **Non-contextual or narrating comments**: Flag comments that restate code or
test names, duplicate
+ rationale, or explain generic purpose (e.g., `# Log for debugging` above
`logging.debug(...)`).
+ Explaining "why" is insufficient unless the comment preserves specific
context that is not
+ readily apparent from the code itself. Identify the misunderstanding or
incorrect change the comment prevents.
+ If removing a comment does not lose context, flag it for removal. A one-line
change can still
+ warrant an explanation of a constraint or failure mode. Apply the
+ [comment guidance in AGENTS.md](../../AGENTS.md#coding-standards).
- **Empty PR descriptions**: PRs with just the template filled in and no
actual description of the changes.
## Quality Signals to Check
diff --git a/AGENTS.md b/AGENTS.md
index 4b67debf840..b8ff11f1efe 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -134,7 +134,14 @@ reported as such are described in "What is NOT considered
a security vulnerabili
- **Always format and check Python files with ruff immediately after writing
or editing them:** `uv run ruff format <file_path>` and `uv run ruff check
--fix <file_path>`. Do this for every Python file you create or modify, before
moving on to the next step.
- No `assert` in production code.
-- **Comment sparingly — code says *what*, comments say *why*.** Add a comment
only when the reasoning is non-obvious and cannot be carried by a clear name or
the code itself. Do not write narrating comments that restate the next line, do
not pad logic with multi-line prose, and do not repeat the same rationale at
several sites — put one concise note at the source of truth and let the others
stand on their own. Tests whose names already describe intent need no
explanatory comment. Reserve [...]
+- **Comment only when context is not readily apparent from the code.**
+ Generic purpose explanation do not qualify: `# Log for debugging` above
`logging.debug(...)`,
+ `# Validate for safety`, `# Retry for reliability` and similar provide no
useful information.
+ Useful comments record a specific constraint, invariant, compatibility
quirk, or tradeoff.
+ Before adding a comment, identify the misunderstanding or incorrect change
it would prevent.
+ If removing a comment will lose non-code-related context, omit it. Do not
narrate code, repeat code,
+ or invent a rationale in comments. Keep necessary explanations short,
precise and at the source of truth.
+ Do not repeat comments at each call site. Judge each comment value by the
preserved context only.
- `time.monotonic()` for durations, not `time.time()`.
- In `airflow-core`, functions with a `session` parameter must not call
`session.commit()`. Use keyword-only `session` parameters.
- Imports at top of file. Valid exceptions: circular imports, lazy loading for
worker isolation, `TYPE_CHECKING` blocks.
diff --git a/dev/skill-evals/last-eval-hash.txt
b/dev/skill-evals/last-eval-hash.txt
index d7d5ed6e509..d3cf53abff5 100644
--- a/dev/skill-evals/last-eval-hash.txt
+++ b/dev/skill-evals/last-eval-hash.txt
@@ -1,3 +1,3 @@
# Generated by dev/skill-evals/eval.py — do not edit or resolve conflicts by
hand.
# Run `prek run run-skill-eval --hook-stage manual --all-files` to regenerate.
-0931cd86d4490f26696e6a5f5399060666f748d5ed9cb30db0cbe9f84849fddf
+485f916101818db48c8311bd3e96770695e8681411e9c063a1ff05bc1d051d7c
