docs: refactor AGENTS.md into an entry point, move build mechanics to CONTRIBUTING - #1045
Conversation
… CONTRIBUTING Bring AGENTS.md in line with the AGENTS.md convention introduced for the spec repo in substrait-io/substrait#1117: a lean entry point plus the notes agents specifically need, with generally-applicable build and contribution mechanics living in the human-facing docs. - Restructure the AGENTS.md intro into a "Start here" pointer to README.md and CONTRIBUTING.md. - Add a "Keep PR descriptions high-signal" guideline: drop file lists, CI-verified-pass claims, and implicit process notes; keep commit bodies free of git trailers. - Move the build/test/format command reference, PMD tripwires, and the build-logic included-build note into a new CONTRIBUTING.md "Building and testing" section, and fold the JDK 17 daemon-consistency and GraalVM native-image details into its "Gradle & JDK 17" section. AGENTS.md keeps only the agent-specific verification workflow.
benbellick
left a comment
There was a problem hiding this comment.
Thanks! I think this is a great improvement. There may be some more places worth slimming down, but we can save it for a subsequent PR. 🚀
|
|
||
| For GitHub work (issues, PRs), use the `gh` CLI. | ||
|
|
||
| ## What this project is |
There was a problem hiding this comment.
Also think we don't really need this now, it should just be in the README if it isn't already.
| ## Building and testing | ||
|
|
||
| The build / test / format command reference, the PMD ruleset and its tripwires, the JDK 17 | ||
| daemon consistency rules, and the GraalVM native-image toolchain all live in | ||
| [`CONTRIBUTING.md`](CONTRIBUTING.md#building-and-testing). Two habits matter most for agents: | ||
|
|
||
| - **Build the whole thing before pushing.** Narrower local tasks pass while CI fails: **PMD** | ||
| runs only via `check`/`build` and `javadoc` doclint only via `build`/`javadocJar` — never | ||
| via `compileJava` / `test` / `spotlessCheck` — while CI runs the full `./gradlew build | ||
| --rerun-tasks`. Run `./gradlew :core:check :core:javadoc` (and the module you touched) | ||
| before pushing. | ||
| - **When you extend the public expression/visitor API, verify the dependent modules still | ||
| compile** — they have their own visitor implementors: | ||
| `./gradlew :core:spotlessCheck :isthmus:compileJava :spark:spark-3.5_2.12:compileScala :examples:substrait-spark:compileJava` |
There was a problem hiding this comment.
| ## Building and testing | |
| The build / test / format command reference, the PMD ruleset and its tripwires, the JDK 17 | |
| daemon consistency rules, and the GraalVM native-image toolchain all live in | |
| [`CONTRIBUTING.md`](CONTRIBUTING.md#building-and-testing). Two habits matter most for agents: | |
| - **Build the whole thing before pushing.** Narrower local tasks pass while CI fails: **PMD** | |
| runs only via `check`/`build` and `javadoc` doclint only via `build`/`javadocJar` — never | |
| via `compileJava` / `test` / `spotlessCheck` — while CI runs the full `./gradlew build | |
| --rerun-tasks`. Run `./gradlew :core:check :core:javadoc` (and the module you touched) | |
| before pushing. | |
| - **When you extend the public expression/visitor API, verify the dependent modules still | |
| compile** — they have their own visitor implementors: | |
| `./gradlew :core:spotlessCheck :isthmus:compileJava :spark:spark-3.5_2.12:compileScala :examples:substrait-spark:compileJava` |
Do we need all of that since its already in the CONTRIBUTING.md? If we find the agent isn't doing this itself, then we can add a single line that says:
run all tests and ensure they are passing before pushing
| @@ -199,6 +181,19 @@ compile — they have their own visitor implementors: | |||
| valid commit message). Scope tags seen in history: `feat(core)`, `feat(pojo)`, | |||
There was a problem hiding this comment.
I also think this whole section can reasonably be removed. Its all stuff you'd want a human to know as well, and there is a section called Commit Conventions already in CONTRIBUTING.md.
| - **Lists of files touched** — they're in the diff. | ||
| - **Claims that CI-verified things pass** — e.g. "tests pass", "spotless clean". If they | ||
| didn't, the checks would be red. | ||
| - **Process notes that are already implicit** — e.g. "opened as draft pending review". |
Drop the "What this project is" section, which the readme already covers, and the conventional-commits bullet, which duplicates the Commit Conventions section in CONTRIBUTING.md. Collapse the build/test notes to a single build-before-pushing line pointing at CONTRIBUTING.md. Point the readme links at readme.md; the file is lowercase, so the README.md links resolved locally on case-insensitive filesystems but 404 on GitHub.
Aligns this repo's
AGENTS.mdwith the ecosystem convention introduced for the spec repo in substrait-io/substrait#1117: a lean entry point plus the notes agents specifically need, with generally-applicable build and contribution mechanics living in the human-facing docs so there is a single source of truth.What changes
AGENTS.mdbecomes an entry point. The intro is now a "Start here" pointer toREADME.mdandCONTRIBUTING.mdrather than a standalone build guide.semantic-releasecommit message and feedCHANGELOG.md.CONTRIBUTING.md. The build/test/format command reference, PMD ruleset and tripwires, and thebuild-logicincluded-build note land in a new "Building and testing" section; the JDK 17 daemon-consistency and GraalVM native-image details fold into the existing "Gradle & JDK 17" section.AGENTS.mdkeeps only the agent-specific verification workflow (build-before-push, cross-module compile check).The codebase-specific architecture knowledge (module layout, Immutables/visitor/proto-converter patterns, Isthmus/Spark change-safety notes) stays in
AGENTS.md— it is this repo's equivalent of the spec PR's "what agents need to get right." No content was lost; the build mechanics were relocated, not removed.🤖 Generated with AI