-
Notifications
You must be signed in to change notification settings - Fork 2
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
Created base project #2
Conversation
And testing of the dockerfile
- name: Run lint | ||
run: yarn lint | ||
- name: Build the project | ||
run: yarn build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would put build
before tests
- if the project doesn't build, it probably won't pass the tests.
- name: Install dependencies | ||
run: yarn install --frozen-lockfile | ||
- name: Run tests | ||
run: yarn test | ||
- name: Run lint | ||
run: yarn lint | ||
- name: Build the project | ||
run: yarn build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you consider making these separate jobs? This way tests, linting and building would run in parallel and, most importantly, they will show a bigger picture.
Consider three scenarios:
- Tests failed, codestyle and build status unknown
- Tests, codestyle, build — all three jobs have failed status
- Tests are red, build and codestyle are green
Two latter cases allow you to draw some conclusions about what's wrong even without reading the output. But they would be hidden behind tests failure in a sequential run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you have an example of this workflow?
I understand your point, but I also believe that it is objective is to work more as a general check to remind you to run those three things than to be the source of linting/test problems (so if you see that the linting failed, you manually run it + tests + build on your own machine).
Separating them is not a problem, but I would need to assign three separate instance.
Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separating them is not a problem, but I would need to assign three separate instance.
Worth it in my book.
but I also believe that it is objective is to work more as a general check to remind you to run those three things than to be the source of linting/test problems (so if you see that the linting failed, you manually run it + tests + build on your own machine).
That's your way. And it will be perfectly possible with my suggestion.
I like PR checks to tell me more info about what's broken without making me run all the checks locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to #8
Please see if that implementation aligns to what you are referring.
If it doesn't build, then it won't be able to test
This commit closes #1
Created base files, including the package.json, yarn.lock, minimal CI requirements, CODEOWNERS file and action.yml file.
This will be the boilerplate upon we will be working on.