-
Notifications
You must be signed in to change notification settings - Fork 0
[ARCHIVED] Benchmark issue 471 #471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,6 +62,10 @@ All git forge operations (GitHub API calls, PR comments, issue creation, workflo | |
|
|
||
| **When reviewing PRs:** Flag any direct `exec.Command("gh", ...)`, raw GitHub API calls, or other forge-specific operations outside `internal/forge/github/` as a medium-severity or higher finding. This is an architectural violation, not a style preference. | ||
|
|
||
| ## API pattern replication | ||
|
|
||
| When implementing a new method that calls the same API endpoint or uses the same multi-step API pattern as an existing method in the file, read the existing implementation first. Replicate its error handling (e.g., truncation checks, 404-to-ErrNotFound mapping), retry logic (e.g., `retryOnTransient`), and response validation. In `internal/forge/github/github.go`, `commitFilesTo` implements the Git Trees API pattern (refs → commit → tree) with truncation checks — any new method using the same endpoint must handle truncation consistently. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] specificity_level The new section names specific functions (commitFilesTo) and API details (Git Trees API), which is more granular than some sections. However, this is consistent with existing sections — the Go code section names specific files and the Forge abstraction section references specific commands. No change warranted. |
||
|
|
||
| ## Architecture Decision Records (ADRs) | ||
|
|
||
| These rules apply whenever you touch `docs/ADRs/` or review a PR that does. Full authoring guidance is in [`skills/writing-adrs/SKILL.md`](skills/writing-adrs/SKILL.md); invoke that skill when writing a new ADR. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[medium] protected-path
This PR modifies AGENTS.md, which is a protected governance file. The change is well-justified (linked to issue #470 with clear rationale from a real incident on PR fullsend-ai#2360), but human approval is always required for protected-path changes regardless of context.