-
Notifications
You must be signed in to change notification settings - Fork 8
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
ci: nightly integration test runs #103
Conversation
f10c480
to
f9b206b
Compare
@@ -21,76 +22,3 @@ jobs: | |||
- run: yarn install | |||
- run: yarn build | |||
- run: yarn test | |||
integration: |
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.
Whe removed from here?
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.
To make it a separate workflow that can run daily. Don't need the lint and unit jobs to run daily.
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.
Don't we want integration run on a PR? Assuming pr.yml triggeres on PR creation/push
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.
on:
pull_request:
determines whether it runs on a pull request or not, and its part on the integration.yml file. Filename can be anything. As you can see integration workflow being triggered on this PR right now.
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 missed on: pull_request hook in integration.yaml
We can keep one file and conditionally trigger lint and unit jobs (when github.event is pullrequest).
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.
Well it's just a suggestion but not a blocker.
Approving
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.
Nice suggestion. I'll cater to it whenever Im doing a cleanup/refactor.
Closes: #100
Notification test (by removing the failure condition)