Skip to content

Docs: Clarify code style checks before commit in AGENTS.md#15822

Open
manuzhang wants to merge 1 commit intoapache:mainfrom
manuzhang:docs-agents-codestyle-check
Open

Docs: Clarify code style checks before commit in AGENTS.md#15822
manuzhang wants to merge 1 commit intoapache:mainfrom
manuzhang:docs-agents-codestyle-check

Conversation

@manuzhang
Copy link
Copy Markdown
Member

No description provided.

Co-authored-by: Codex <codex@openai.com>
- One concern per PR. Unrelated whitespace, import, or formatting changes go in separate PRs.
- Keep first version of a PR minimal — defer recovery, optimization, and edge cases to follow-ups.
- Commit messages describe the *what* and *why*, not implementation details.
- Before creating or amending a commit, run the relevant code style check and fix any violations.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a pre commit hook, I feel like adding it here is applying an agent in an inefficient way. I can post an example pre commit hook if people like, but it's much better imho to phase a precommit that just runs spotless apply and restages

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption is an agent going to be run and read this document anyway. Is it more efficient for an agent to check pre-commit output than run style check?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for precommit hooks its helpful not just for agents for actual humans too !

Copy link
Copy Markdown
Member

@RussellSpitzer RussellSpitzer Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manuzhang The idea here is that if you put it a pre-commit hook, the agent doesn't have to have any instructions, it will always just get cleaned up automatically.

Anything we add to this file is essentially extra token cost every single time you go to the LLM. This is a two fold problem.

  1. We reduce the context space for valuable context
  2. We increase the cost of using our agent.md

If we leave it in agent.md we are essentially just wasting tokens telling it to do something which can programmatically be invoked without the agent having to be aware of it.

So if instead you do something like

#!/bin/sh
#
# Pre-commit hook that runs spotless apply to format code before committing
#
# Check if this is the Apache Iceberg project
if [ ! -f "build.gradle" ] || ! grep -q 'group = "org.apache.iceberg"' build.gradle 2>/dev/null; then
    exit 0
fi
# Capture the list of staged files before spotless modifies them
STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM)
if [ -z "$STAGED_FILES" ]; then
    exit 0
fi
echo "Running spotlessApply..."
./gradlew -DallModules spotlessApply
if [ $? -ne 0 ]; then
    echo "ERROR: spotlessApply failed. Please fix the issues and try again."
    exit 1
fi
# Re-stage only the files that were originally staged (in case spotless reformatted them)
echo "$STAGED_FILES" | xargs git add
echo "spotlessApply complete."
exit 0

You don't need to tell your agent anything at all it just always runs spotless apply when you commit

_- Note I use a lot of worktrees and clones so I use the above hook globally just so whenever i'm running on any iceberg repo it does the spotless.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RussellSpitzer That looks neat! Do you plan to add it to the repo? I'm looking for an approach that can be automatically built into development process.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works with pre-commit, we could add it to the repo but you would still need to install pre-commit first. I thought folks may be more opinionated about their env, so I didn't add it to the repo. It's very easy to setup though, and once you do, every time you commit it just gets in the way, runs spotless, stages and commits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants