-
Notifications
You must be signed in to change notification settings - Fork 0
docs: realign CI review loop to actual fork integrations #12
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 1 commit
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,26 +23,58 @@ When asked to push code, follow this full workflow: | |||||
| 0. ensure that you are on a branch that is specific to this change i.e feat/new-feature or fix/bug-fix or docs/update-readme or chore/update-dependencies, or test/new-test, etc | ||||||
| 1. Push the branch to the remote (`git push -u origin <branch>`) | ||||||
| 2. Create a PR using `gh pr create`. Ensure that you are adhering to the PR template. **Do NOT include "Generated with Claude Code" or any AI attribution in PR descriptions.** | ||||||
| 3. Wait for Gemini to review the PR (poll with `gh pr checks` and `gh api repos/{owner}/{repo}/pulls/{number}/reviews` until a review appears) | ||||||
| 4. Once a review exists, pull down the review comments: `gh api repos/{owner}/{repo}/pulls/{number}/comments` and `gh api repos/{owner}/{repo}/pulls/{number}/reviews` | ||||||
| 5. Address every review comment — fix the code, re-run the quality gates (tests, lint, tsc). Resolve the comment appropriately and post that on the PR directly. | ||||||
| 6. Push the fixes | ||||||
| 7. Report what was changed in response to the review | ||||||
| 3. Wait for GitHub Actions CI to start. Poll with `gh pr checks <pr>` until all four jobs (`typecheck`, `test`, `lint`, `android-build`) report a status. If any fail, fix and re-push before reading reviewer comments. | ||||||
| 4. Once CI is green, wait for Gemini to post (`gh api repos/{owner}/{repo}/pulls/{number}/comments` + `.../reviews`). | ||||||
| 5. Address every Gemini review comment — fix the code, or reply on the thread explaining why it's fine. Resolve the conversation either way. | ||||||
| 6. Push the fixes; pre-commit gates re-run. Comment `/gemini review` to re-trigger Gemini. | ||||||
| 7. Loop until CI is green and Gemini has nothing blocking. | ||||||
| 8. Report what was changed in response to the review. | ||||||
|
|
||||||
| ## CI Review Loop | ||||||
|
|
||||||
| The repo has three automated reviewers on every PR. After pushing, loop until all are green: | ||||||
| After pushing, loop until everything below is green or addressed. | ||||||
|
|
||||||
| | Reviewer | What it checks | How to address | | ||||||
| |---|---|---| | ||||||
| | **Gemini Bot** | Code quality, style, logic issues | Read comments via `gh api`, fix code or reply explaining why it's fine, then comment `/gemini review` to trigger a fresh pass | | ||||||
| | **Codecov** | Test coverage thresholds | Add missing tests, ensure new code is covered. Check the Codecov report for uncovered lines | | ||||||
| | **SonarCloud** | Security hotspots, code smells, duplications, bugs | Fix flagged issues — especially security hotspots and duplications. Resolve quality gate failures before merging | | ||||||
| ### GitHub Actions CI (merge-blocking) | ||||||
|
|
||||||
| The `CI` workflow runs four jobs on every push and PR targeting `main` or `wildlife-reid`: | ||||||
|
|
||||||
| | Job | What it checks | | ||||||
| |---|---| | ||||||
| | `typecheck` | `tsc --noEmit` | | ||||||
| | `test` | Jest unit + integration tests with coverage | | ||||||
| | `lint` | ESLint + `gradlew :app:lintDebug` + SwiftLint | | ||||||
| | `android-build` | Full debug Gradle build | | ||||||
|
|
||||||
| If any job fails, fix locally (re-run `npx tsc --noEmit && npm test && npm run lint` to mirror CI), push, and wait for the next run. | ||||||
|
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. There is a discrepancy between the CI jobs described in the table and the suggested local "mirror" command.
To accurately mirror the CI process and save time locally, consider suggesting
Suggested change
Author
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. Fixed in 5819fc1 — replaced the single mirror-command line with a per-job table. |
||||||
|
|
||||||
| ### Gemini Code Assist (advisory) | ||||||
|
|
||||||
| Auto-reviews every PR on open and on `/gemini review`. Posts a summary comment plus line-level review comments tagged by severity. | ||||||
|
|
||||||
| **Workflow:** | ||||||
| 1. Push code → wait for all three reviewers to report | ||||||
| 2. Pull down Gemini comments, Codecov report, and SonarCloud findings | ||||||
| 3. Fix issues: code changes for Gemini/SonarCloud, add tests for Codecov | ||||||
| 4. Re-run local quality gates (`npm run lint && npm test && npx tsc --noEmit`) | ||||||
| 5. Push fixes, comment `/gemini review` on the PR to re-trigger Gemini | ||||||
| 6. Repeat until all three reviewers pass with no blocking issues | ||||||
| 1. Push → wait for the Gemini summary + comments to land (~1-2 min after PR open or `/gemini review`). | ||||||
| 2. Pull down comments: `gh api repos/{owner}/{repo}/pulls/{number}/comments` and `.../reviews`. | ||||||
| 3. Address every comment — fix the code, or reply on the comment thread explaining why it's a non-issue. Resolve the conversation. | ||||||
| 4. Re-run pre-commit gates locally, push fixes. | ||||||
| 5. Comment `/gemini review` on the PR to trigger a fresh pass. | ||||||
| 6. Repeat until Gemini's findings are addressed. | ||||||
|
|
||||||
| Gemini findings are advisory — they don't block merge themselves, but unaddressed legitimate findings should block merge in human review. | ||||||
|
|
||||||
| ### Codex 5.5 (on-demand second opinion) | ||||||
|
|
||||||
| Use for scoped deep reviews when warranted (foundational PRs, risky refactors, pre-merge sanity passes). Not part of the routine PR loop. | ||||||
|
|
||||||
| ```bash | ||||||
| codex exec -s read-only --skip-git-repo-check "<scoped review prompt>" | ||||||
| ``` | ||||||
|
|
||||||
| File substantive findings to `kb/wildlife-reid-mobile/outputs/reports/`. | ||||||
|
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. The path
Suggested change
Author
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. Rejecting this suggestion. The kb directory Also, the directory already exists with substantive content (~20 wiki/raw/output files); renaming would break internal cross-references and stored memory. The pattern-match Gemini did here (fork name in package.json → KB path) doesn't apply to this project's intentional domain-vs-app naming. |
||||||
|
|
||||||
| ### Human review (final gate) | ||||||
|
|
||||||
| Final approval required before merging into `wildlife-reid` or `main`. | ||||||
|
|
||||||
| ### Not currently installed | ||||||
|
|
||||||
| Codecov and SonarCloud are referenced in some upstream docs but are **not** wired up on this fork. If reinstated later, document them here. CodeQL (free, one-click at `Settings → Code security`) is a reasonable alternative if SAST is desired. | ||||||
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.
The placeholder
<pr>is inconsistent with the{number}placeholder used in subsequent steps (lines 27 and 56). For consistency and to ensure automated tools like Claude Code can correctly identify the variable, it is better to use{number}throughout. Additionally,gh pr checkstypically works without arguments for the current branch's PR, which might be simpler for the user.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.
Fixed in 5819fc1 — replaced
<pr>with{number}and added the no-arg variant for clarity. Thanks.