mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
chore: polish PR review skills
This commit is contained in:
@@ -110,7 +110,7 @@ Before any substantive review or prep work, **always rebase the PR branch onto c
|
|||||||
- During `prepare-pr`, use concise, action-oriented subjects **without** PR numbers or thanks; reserve `(#<PR>) thanks @<pr-author>` for the final merge/squash commit.
|
- During `prepare-pr`, use concise, action-oriented subjects **without** PR numbers or thanks; reserve `(#<PR>) thanks @<pr-author>` for the final merge/squash commit.
|
||||||
- Group related changes; avoid bundling unrelated refactors.
|
- Group related changes; avoid bundling unrelated refactors.
|
||||||
- Changelog workflow: keep the latest released version at the top (no `Unreleased`); after publishing, bump the version and start a new top section.
|
- Changelog workflow: keep the latest released version at the top (no `Unreleased`); after publishing, bump the version and start a new top section.
|
||||||
- When working on a PR: add a changelog entry with the PR number and thank the contributor (mandatory in this workflow).
|
- When working on a PR: add a changelog entry line with the PR number `(#<PR>)` and `thanks @<pr-author>` when author metadata is available (mandatory in this workflow).
|
||||||
- When working on an issue: reference the issue in the changelog entry.
|
- When working on an issue: reference the issue in the changelog entry.
|
||||||
- In this workflow, changelog is always required even for internal/test-only changes.
|
- In this workflow, changelog is always required even for internal/test-only changes.
|
||||||
|
|
||||||
|
|||||||
@@ -41,11 +41,12 @@ scripts/pr-merge <PR>
|
|||||||
scripts/pr-merge run <PR>
|
scripts/pr-merge run <PR>
|
||||||
```
|
```
|
||||||
|
|
||||||
3. Ensure output reports:
|
3. Capture and report these values in a human-readable summary (not raw `key=value` lines):
|
||||||
|
|
||||||
- `merge_sha=<sha>`
|
- Merge commit SHA
|
||||||
- `merge_author_email=<email>`
|
- Merge author email
|
||||||
- `comment_url=<url>`
|
- Merge completion comment URL
|
||||||
|
- PR URL
|
||||||
|
|
||||||
## Steps
|
## Steps
|
||||||
|
|
||||||
@@ -97,3 +98,4 @@ Cleanup is handled by `run` after merge success.
|
|||||||
|
|
||||||
- End in `MERGED`, never `CLOSED`.
|
- End in `MERGED`, never `CLOSED`.
|
||||||
- Cleanup only after confirmed merge.
|
- Cleanup only after confirmed merge.
|
||||||
|
- In final chat output, use labeled lines or bullets; do not paste raw wrapper diagnostics unless debugging.
|
||||||
|
|||||||
@@ -74,6 +74,11 @@ jq -r '.changelog' .local/review.json
|
|||||||
jq -r '.docs' .local/review.json
|
jq -r '.docs' .local/review.json
|
||||||
```
|
```
|
||||||
|
|
||||||
|
Changelog gate requirement:
|
||||||
|
|
||||||
|
- `CHANGELOG.md` must include a newly added changelog entry line.
|
||||||
|
- When PR author metadata is available, that same changelog entry line must include `(#<PR>) thanks @<pr-author>`.
|
||||||
|
|
||||||
4. Commit scoped changes
|
4. Commit scoped changes
|
||||||
|
|
||||||
Use concise, action-oriented subject lines without PR numbers/thanks. The final merge/squash commit is the only place we include PR numbers and contributor thanks.
|
Use concise, action-oriented subject lines without PR numbers/thanks. The final merge/squash commit is the only place we include PR numbers and contributor thanks.
|
||||||
|
|||||||
54
scripts/pr
54
scripts/pr
@@ -625,12 +625,54 @@ prepare_validate_commit() {
|
|||||||
echo "commit subject validated: $subject"
|
echo "commit subject validated: $subject"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
validate_changelog_entry_for_pr() {
|
||||||
|
local pr="$1"
|
||||||
|
local contrib="$2"
|
||||||
|
|
||||||
|
local added_lines
|
||||||
|
added_lines=$(git diff --unified=0 origin/main...HEAD -- CHANGELOG.md | awk '
|
||||||
|
/^\+\+\+/ { next }
|
||||||
|
/^\+/ { print substr($0, 2) }
|
||||||
|
')
|
||||||
|
|
||||||
|
if [ -z "$added_lines" ]; then
|
||||||
|
echo "CHANGELOG.md is in diff but no added lines were detected."
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
local pr_pattern
|
||||||
|
pr_pattern="(#$pr|openclaw#$pr)"
|
||||||
|
|
||||||
|
local with_pr
|
||||||
|
with_pr=$(printf '%s\n' "$added_lines" | rg -in "$pr_pattern" || true)
|
||||||
|
if [ -z "$with_pr" ]; then
|
||||||
|
echo "CHANGELOG.md update must reference PR #$pr (for example, (#$pr))."
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
if [ -n "$contrib" ] && [ "$contrib" != "null" ]; then
|
||||||
|
local with_pr_and_thanks
|
||||||
|
with_pr_and_thanks=$(printf '%s\n' "$added_lines" | rg -in "$pr_pattern" | rg -i "thanks @$contrib" || true)
|
||||||
|
if [ -z "$with_pr_and_thanks" ]; then
|
||||||
|
echo "CHANGELOG.md update must include both PR #$pr and thanks @$contrib on the changelog entry line."
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
echo "changelog validated: found PR #$pr + thanks @$contrib"
|
||||||
|
return 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
echo "changelog validated: found PR #$pr (contributor handle unavailable, skipping thanks check)"
|
||||||
|
}
|
||||||
|
|
||||||
prepare_gates() {
|
prepare_gates() {
|
||||||
local pr="$1"
|
local pr="$1"
|
||||||
enter_worktree "$pr" false
|
enter_worktree "$pr" false
|
||||||
|
|
||||||
checkout_prep_branch "$pr"
|
checkout_prep_branch "$pr"
|
||||||
bootstrap_deps_if_needed
|
bootstrap_deps_if_needed
|
||||||
|
require_artifact .local/pr-meta.env
|
||||||
|
# shellcheck disable=SC1091
|
||||||
|
source .local/pr-meta.env
|
||||||
|
|
||||||
local changed_files
|
local changed_files
|
||||||
changed_files=$(git diff --name-only origin/main...HEAD)
|
changed_files=$(git diff --name-only origin/main...HEAD)
|
||||||
@@ -647,6 +689,8 @@ prepare_gates() {
|
|||||||
echo "Missing CHANGELOG.md update in PR diff. This workflow requires a changelog entry."
|
echo "Missing CHANGELOG.md update in PR diff. This workflow requires a changelog entry."
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
local contrib="${PR_AUTHOR:-}"
|
||||||
|
validate_changelog_entry_for_pr "$pr" "$contrib"
|
||||||
|
|
||||||
run_quiet_logged "pnpm build" ".local/gates-build.log" pnpm build
|
run_quiet_logged "pnpm build" ".local/gates-build.log" pnpm build
|
||||||
run_quiet_logged "pnpm check" ".local/gates-check.log" pnpm check
|
run_quiet_logged "pnpm check" ".local/gates-check.log" pnpm check
|
||||||
@@ -1043,10 +1087,14 @@ EOF_COMMENT
|
|||||||
git branch -D "pr-$pr" 2>/dev/null || true
|
git branch -D "pr-$pr" 2>/dev/null || true
|
||||||
git branch -D "pr-$pr-prep" 2>/dev/null || true
|
git branch -D "pr-$pr-prep" 2>/dev/null || true
|
||||||
|
|
||||||
|
local pr_url
|
||||||
|
pr_url=$(gh pr view "$pr" --json url --jq .url)
|
||||||
|
|
||||||
echo "merge-run complete for PR #$pr"
|
echo "merge-run complete for PR #$pr"
|
||||||
echo "merge_sha=$merge_sha"
|
echo "merge commit: $merge_sha"
|
||||||
echo "merge_author_email=$selected_merge_author_email"
|
echo "merge author email: $selected_merge_author_email"
|
||||||
echo "comment_url=$comment_url"
|
echo "completion comment: $comment_url"
|
||||||
|
echo "$pr_url"
|
||||||
}
|
}
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
|
|||||||
Reference in New Issue
Block a user