Add a npx hereby validate command to group all repo validations - #64161
Add a npx hereby validate command to group all repo validations#64161Wesley Wigham (weswigham) wants to merge 1 commit into
npx hereby validate command to group all repo validations#64161Conversation
…lidations you otherwise do
typescript-automation
Bot
added
the
For Uncommitted Bug
|
|
||
| await runValidation("test", async () => { | ||
| await runTests(); | ||
| await runTestExtension(); |
There was a problem hiding this comment.
Can we take the time now to get these extension tests out too into their own task? I've been wanting hereby test to not run them.
| } | ||
|
|
||
| export const validate = task({ | ||
| name: "validate", |
There was a problem hiding this comment.
We have a lot of terms, test, check, validate... Sort of wonder if we should somehow name this with "all" in the name to make it very clear what must be done
| npx hereby test # Run tests | ||
| npx hereby lint # Run linters | ||
| npx hereby format # Format the code | ||
| npx hereby validate # Build, test, lint, and format the project |
There was a problem hiding this comment.
One of the things I want to do in this file is to make it clear that the build, test, lint, just matter when editing the tsc dir.
We can probably do that in a followup, as the "CRITICAL" language here should really be in the CCA file instead
There was a problem hiding this comment.
🟡 Changes recommended
Build must use the failure-capturing path so all validations run and failures are aggregated.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a unified npx hereby validate command for repository checks.
Changes:
- Adds validation orchestration with optional API tests and aggregated failures.
- Updates Copilot instructions to use the consolidated workflow.
File summaries
| File | Description |
|---|---|
Herebyfile.mjs |
Implements the validation task and --api option. Build failures currently prevent remaining validations from running. |
.github/copilot-instructions.md |
Documents the unified validation command and checklist. |
Review details
Suppressed comments (1)
Herebyfile.mjs:1371
- With
--api, this callsrunTestAPIdirectly but never runs thebuildAPITestsprerequisite that the existingtest:apitask declares at lines 1250-1254. On a clean checkout, the generated API test artifacts therefore are not prepared, so the documented replacement fortest:apican fail or exercise stale output. Add that prerequisite conditionally when API validation is requested.
dependencies: [build],
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| export const validate = task({ | ||
| name: "validate", | ||
| description: "Builds, tests, lints, and formats the repo. Pass --api to include API tests.", | ||
| dependencies: [build], |
There was a problem hiding this comment.
I didn't notice this, but yeah build doesn't need to be a dep, right? nothing needs that to have happened? I guess the API does?
There was a problem hiding this comment.
The API needs it to work and I have doubts on if lint, test, and format would reliably work in the presence of a non-functioning build.
There was a problem hiding this comment.
So right, I was just surprised by it being a dep versus another func call, but it doesn't practically matter.
I have seen a lot of very dumb powershell trying to shorten the list of commands into "a single script" with appropriate error handling, and a lot of the time it does not work.
Better to just provide it upfront as a single command.