Conversation
maxn990
left a comment
There was a problem hiding this comment.
Nice Swar! Left a few comments, but overall looks awesome. Looks like there are some formatting issues too, so just make sure to run prettier :)
| } as unknown as SelectQueryBuilder<Order>; | ||
| beforeEach(async () => { | ||
| // Run all migrations fresh for each test | ||
| await testDataSource.runMigrations(); |
There was a problem hiding this comment.
Could we clear the data and keep the schema instead of running the migrations every time? This could be quite slow
There was a problem hiding this comment.
Ok so Dalton told me why we couldn't do this. The migration that seeds the db has migrations that run on top of it. I think it should be fine since your migrations run really fast.
package.json
Outdated
| "prettier:check": "prettier --check \"**/*.{ts,tsx,js,jsx,json,md}\"", | ||
| "prettier:write": "prettier --write \"**/*.{ts,tsx,js,jsx,json,md}\"" |
There was a problem hiding this comment.
Adding a more specific path like "apps/" could help to exclude node_modules and dist, etc since we don't need to check those every time
maxn990
left a comment
There was a problem hiding this comment.
niceeee looks really good!! one super small comment i missed the first time then i think we'll be ready to go
| - run: yarn install --frozen-lockfile | ||
| - run: yarn list strip-ansi string-width string-length | ||
| - run: yarn test No newline at end of file | ||
| - run: npx jest No newline at end of file |
There was a problem hiding this comment.
i think these things should be moved to jobs from services
ℹ️ Issue
Closes 132
📝 Description
Added prettier verification in our CI/CD pipeline so it runs on pushed commits and fails when there are formatting violations, and passes when they are fixed.
✔️ Verification
Tested by running yarn prettier:check and pushing a commit that I knew had formatting violations to make sure the workflow failed. Then I commited and pushed fixes by running yarn prettier:write and made sure the workflow passed since there were no violations anymore.