docs: clarify build and contribution conventions for agents - #910
docs: clarify build and contribution conventions for agents#910zhjwpku wants to merge 2 commits into
Conversation
Keep CMake and Meson definitions aligned and make contributions easier to review with consistent commit messages and pre-commit checks.
| ## PR & Commit Conventions | ||
|
|
||
| - Use Conventional Commits for commit messages. | ||
| - Commit messages describe the what and why, not implementation details. |
There was a problem hiding this comment.
I would recommend keeping PR title and description in sync with latest commit. And then we can directly use those to be the final git commit message.
There was a problem hiding this comment.
This line comes from https://github.com/apache/iceberg/blob/main/AGENTS.md?plain=1#L169, I'm not sure how can the AI coding tools know which is the latest commit, there can be other comments to be addressed after the latest commit.
There was a problem hiding this comment.
I think @wgtmac's concern is that PR title and description could be out of sync when the PR contents are updated based on reviews. On the other hand, I'm not worried about Commit message itself, where AI has always done a good job in describing what and why.
There was a problem hiding this comment.
Yeah, I agree that the PR title and description can get out of sync, but I'm not sure how we could reliably instruct the agent to update them once the PR is ready to merge. I think that's a point where human review is still necessary.
I will delete this one since llm should be trained to do a good job.
|
|
||
| - Use Conventional Commits for commit messages. | ||
| - Commit messages describe the what and why, not implementation details. | ||
| - Run `pre-commit` for every PR and fix any reported issues. |
There was a problem hiding this comment.
I think this is enforced by ci so we don't need to mention it?
There was a problem hiding this comment.
Yeah, this is enforced by ci, but I think it would be great if the agent can run pre-commit locally before creating the PR.
There was a problem hiding this comment.
Maybe the agent is capable enough to detect .pre-commit-config.yaml and run the pre-commit automatically. If you still think this is unnecessary, I'll remove it.
There was a problem hiding this comment.
Maybe the agent is capable enough to detect .pre-commit-config.yaml and run the pre-commit automatically.
no, it doesn't. My PR often fails due to pre-commit CI failure.
AI has always done a good job in describing what and why.
3 participants
Keep CMake and Meson definitions aligned and make contributions easier to review with consistent commit messages and pre-commit checks.