Skip to content
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

chore: add basic github workflow #208

Merged
merged 6 commits into from
Apr 22, 2024
Merged

chore: add basic github workflow #208

merged 6 commits into from
Apr 22, 2024

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Apr 1, 2024

Towards #56

Introduces a basic github action that runs on non-draft PRs. Currently only does typechecking for both backend and frontend.

Does the following checks for now:

  • all: formatting checks
  • backend: type checks
  • frontend: unit tests

ESLint (for all) and typechecking (for frontend) to be done as a follow-up

@achou11 achou11 requested a review from ErikSin April 1, 2024 17:57
@achou11 achou11 force-pushed the ac/ci-workflow branch 2 times, most recently from fe0e1c3 to dd4b9e8 Compare April 2, 2024 20:23
@achou11 achou11 force-pushed the ac/ci-workflow branch 2 times, most recently from 69b7ea7 to 4547498 Compare April 10, 2024 15:07
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@achou11 achou11 force-pushed the ac/ci-workflow branch 2 times, most recently from c064690 to 4c7e73f Compare April 15, 2024 21:50
Copy link
Contributor

@EvanHahn EvanHahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally subjective, but: I think we should merge this such that CI passes, even if the tests are incomplete. I would do this by running npm run lint:prettier and npm test (which pass) instead of lint:types.

Later, we should fix types & ESLint, then enable them in CI.

Just my opinion. I think this approach is also reasonable.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@achou11
Copy link
Member Author

achou11 commented Apr 22, 2024

I would do this by running npm run lint:prettier and npm test (which pass) instead of lint:types.

any thoughts on running prettier in a separate job, since it applies to both frontend and backend? currently laid out such that there are separates jobs for backend and frontend checks.

@EvanHahn
Copy link
Contributor

any thoughts on running prettier in a separate job, since it applies to both frontend and backend?

No preference as long as there's no way for Prettier mistakes to slip through. Maybe a separate job for tasks that apply to both?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's probably a way to de-duplicate some of the setup steps that are shared by these jobs but can look into it as a follow-up

note to self: look into composite actions

@achou11 achou11 merged commit c1a9f92 into main Apr 22, 2024
3 checks passed
@achou11 achou11 deleted the ac/ci-workflow branch April 22, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants