General Repo Updates #12
Conversation
Ethan-Arrowood
force-pushed
the
general-repo-updates
branch
from
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
typeand adding some missing dependenicesLand this PR with a merge commit so we retain necessary commit history particularly around the formatting changes.