General Repo Updates - #12
Conversation
22713d8 to
7f279b9
Compare
| on: | ||
| push: | ||
| branches: [main] | ||
| pull_request: |
There was a problem hiding this comment.
| pull_request: | |
| pull_request: | |
| types: [opened, synchronize, reopened] |
There was a problem hiding this comment.
Whats the difference compared to the default? The other workflows don't specify these types. Just curious
There was a problem hiding this comment.
It's just something I started doing a couple years ago because a project I worked on would run CI for both push and pull requests, effectively running CI twice. I didn't know GH workflows back then and whoever set up the triggers didn't either! I fixed it by explicitly set the types and branches (main), then things started to behave. So I just looked into what the valid types are and turns out that pened, synchronize, and reopened are the default types! So, nevermind, you can ignore this. :)
| if ( | ||
| config && | ||
| typeof config === 'object' && | ||
| config !== null && |
There was a problem hiding this comment.
This line is unnecessary.
| config !== null && |
There was a problem hiding this comment.
I'd like to refrain from making core changes in this pr; only reason this code is changing is cause of the format. We can make edits like this separately.
| { | ||
| '204': new Response204(), | ||
| } | ||
| ); |
There was a problem hiding this comment.
Holy smokes, the formatter actually made the code more readable!
Co-authored-by: Chris Barber <chris@cb1inc.com>
|
@cap10morgan I'm tagging you for review directly since I'm making changes to some of the unit test code you recently added. Happy to discuss the changes in more detail 😄 |
| "format:write": "prettier --write .", | ||
| "test:integration": "node --test integrationTests/apiTests/tests/testSuite.mjs", | ||
| "test:unit": "npm run build || true; npx mocha unitTests --config unitTests/.mocharc.json" | ||
| "test:unit": "TSX_TSCONFIG_PATH=./unitTests/tsconfig.json npx mocha --config unitTests/.mocharc.json", |
There was a problem hiding this comment.
I was trying to find something like this! I really wanted to split out the test TS config into a separate file like you did but couldn't find a way to tell TSX to use it. Awesome!
| "format:check": "prettier --check .", | ||
| "format:write": "prettier --write .", | ||
| "test:integration": "node --test integrationTests/apiTests/tests/testSuite.mjs", | ||
| "test:unit": "npm run build || true; npx mocha unitTests --config unitTests/.mocharc.json" |
There was a problem hiding this comment.
I wonder if this is going to trip people up when the tests are run against the TS compiled output. Should we mention this somewhere? Maybe in a dev / contributing doc?
There was a problem hiding this comment.
yeah definitely worth mentioning. I can adjust the contributing doc
…idator Mirrors the oauth #75 shape: now that all three reusables exist on ai-review-prompts main (post-#11/#12/#14), close out harper's caller migration in one shot rather than dragging it across PRs. - `claude-mention.yml` → caller of `_claude-mention.yml` - `claude-issue-to-pr.yml` → caller of `_claude-issue-to-pr.yml` - `claude-review.yml` → SHA pin bumped from `bac5e45` to `11872cb` for parity with the other two Both new callers carry the harper-specific repo conventions (oxlint, RocksDB primary, TypeStrip, `dependencies.md`, no `tsc || true` build tolerance) via the reusables' `repo-specific-conventions:` input. Default `pre-commit-validation` matches the harper-style npm-only flow already shipped in the reusables — no override needed. Adds `validate-caller-workflows.yml` — thin caller of `_validate-caller-workflows.yml`. Catches shadow jobs and mutable refs in the caller files. Make this `validate` job a required status check on `main`. Removes harper's now-redundant local files (oauth dropped the same set in #75; the centralized scripts in `ai-review-prompts` are the single source of truth): - `.github/scripts/authorize-claude-workflow.sh` - `.github/scripts/parse-claude-mention.sh` - `.github/scripts/validate-auth-gate-invariants.sh` - `.github/workflows/auth-gate-invariants.yml` Net: 8 files touched (4 deleted + 1 added + 3 modified), +122 / -795. Caller validator passes locally against the all-caller tree. Workflow-modifying-PR caveat applies — same App-token-401 gotcha that's already kept this PR from getting reviewed by Claude. Subsequent harper PRs will be reviewed normally and will exercise the new `validate-caller-workflows.yml` job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 participants
typeand adding some missing dependenicesLand this PR with a merge commit so we retain necessary commit history particularly around the formatting changes.