Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Playwright tests & CI #69
Playwright tests & CI #69
Changes from 108 commits
875b08b
235dd95
93d85e0
5cc3400
ef2303d
fe8dc36
bec5403
8d715e8
7f29638
332a03d
17a681d
61bd73c
73f0c4c
59694d6
13d31fe
7fa2bb9
0fd85ae
ce38902
373b0e9
a01da1b
c9b4309
0c277e5
bbbbcbd
c301d1f
9daf3ee
c3ce85d
a34b2fd
558cc30
4aecd93
9eed9b5
e8115ec
5544f2a
1ca0cf1
361dd9c
9c1fc69
a7df079
e63ab54
b5a0c77
249546e
149b60f
afe6332
ad1e949
bb7e4d9
2a9634b
31e0efb
a5c11a5
e624d57
3f49cbb
bac080d
4f9555c
80c7bd7
7db13e0
96f47e8
fa64db7
8ed10d8
87f9dce
5baf8e6
03de857
50f8f0c
a0e8fbe
9478c2b
9ce19af
18c4620
ef66678
15670c1
316c648
4607c20
285342f
faa05a0
452b354
4ea6652
3810b9b
62f82c8
bd37741
765cbbf
91181d2
ba6e82b
70f9831
671dabd
80cae74
a106ea9
d68f147
eab3e6d
0786df0
1c78b03
256147a
bd9089b
fa9c809
99b17cf
23a6117
ff3650d
73dfda9
0631f9c
ed0b701
6d82d10
2654200
2d73f2f
8c24e13
11bc2c6
392e9fa
be70d70
9c22fa9
f2de9b4
1a3f357
15749c5
5e60999
157352d
8f48df7
f081173
82e3288
7a00395
dd8e223
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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, almost there :D. Since this is your restore-key, and it makes sense, you want to include OS version, you will also want to update your actual key to have runner.os as the first thing after node-modules, otherwise this will never match!
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.
ok I wasn't aware of that, I thought you suggested it that way because it would match any combination of keys, but it makes sense that it is looking for a substring of that key.
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.
Actually it is looking for a prefix I think! So this way it matches the prefix.
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.
Aha so you do this only if cache is missing. Although it probably wouldn't be terrible if we did it again when cache already exists, would probably be a no-op, in the sense that it would go like "... all installed already, nothing to do.", right? @infomiho .
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'm not sure TBH, some extra work would be performed to ensure that what is installed is okay. It would be less, but still some work would be done. If that work is minimal, we can remove the
if
statement. You'd have to check that by running the CI with and without theif
part.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.
We set this env var in
.env.server.example
as well, FYIThere 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 will leave it there in case the user changes their local env so that CI tests don't fail
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.
So what is our reasoning behind this Action -> it keeps assigning this tag to the last commit on
main
, right?I guess that makes sense! It means people doing
wasp new
and just copying this template are getting the same version.WHat happens when we want to update to new Wasp version -> we update the version here in this action, and push it to
main
together with the rest of the changes that update open-saas to this new version, right? Ok yeah, sounds good!Let's add to README something about this, that explains that this is happening and what we have to do to get open-saas to use new Wasp version.
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.
Maybe this change makes more sense a separate PR? 🤷 But a cool change!
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 will make separate PRs for stuff like this nexttime :)
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.
@vincanger you never added anything about this to some top level README, right?
I would certainly do it, as it can be quite surprising that anything that goes to
main
is basically instantly published, that should be explicitly stated.