-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Workflow consistency and refactor #3238
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
Conversation
Deploying wasp-docs-on-main with
|
Latest commit: |
1bec412
|
Status: | ✅ Deploy successful! |
Preview URL: | https://b5db7b09.wasp-docs-on-main.pages.dev |
Branch Preview URL: | https://cprecioso-consistent-workflo.wasp-docs-on-main.pages.dev |
This comment was marked as resolved.
This comment was marked as resolved.
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.
git config --global core.eol lf | ||
- uses: actions/checkout@v4 | ||
with: |
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 thought we fixed this one already. Well good thing we do now.
Co-authored-by: Franjo Mindek <84568328+FranjoMindek@users.noreply.github.com>
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.
Went through the changes, looking good 👍
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.
Looks great! Just two questions
#### How to run in CI | ||
|
||
We set up a GitHub Action to run the tests in CI. See `.github/workflows/waspc-ci.yaml` for details. | ||
We set up a GitHub Action to run the tests in CI. See `.github/workflows/ci-waspc-test.yaml` for details. |
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.
Doesn't this talk about all tests, not just the CI?
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 was thinking it talks about the tests regarding todoApp, no?
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.
Oh, right, didn't see where we were at. IN that case, is ci-waspc-test
the correct workflow to point it to (might be, just double checking)?
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.
it is! that worflow is too big and does a lot of stuff
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.
somebody braver than me might want to take a stab at it some time 😁
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.
TodoApp renaming PR already moves TodoApp stuff into examples workflows.
Best thing is because the abstraction work well, it's just adding the examples/kitchen-sink
name to the matrix.
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.
Perfect
paths
triggers in Github actions #3219In this PR we do multiple things at once:
push
andpull_request
triggers, andpaths
filters.ci.yaml
workflow that calls all the others. This also simplifies therelease.yaml
workflow since it can just callci
before doing its stuff.actions/checkout
andactions/setup-node
to latest version (no breaking changes)actions/checkout
workaround that use the PR head commit instead of the PR merge commitmain
was broken, but the PR was based on a previous non-brokenmain
, so you wouldn't readily know where the breakage was coming from.main
and PR, this won't happen anymoremain
, not the version ofmain
the PR was based off. Testing on the merge commit means it is more probable that we can reuse the warmed cache.WASP_TELEMETRY_DISABLE
to workflows where we had forgot itResult:
We run the exact same CI checks both in PRs and
main
, continuously (hey that's the C in CI!) We do so in the same consistent way, with easy-to-understand entry points.Future improvement if we feel any pain: