GHA Workflow Refactoring #162
Conversation
Sushisource
force-pushed
the
gh-actions-refactoring
branch
13 times, most recently
from
d54875e to
2a1f7d2
Compare
Sushisource
force-pushed
the
gh-actions-refactoring
branch
from
d92231f to
0833f6e
Compare
Sushisource
force-pushed
the
gh-actions-refactoring
branch
from
29a7e5a to
f5186bf
Compare
Sushisource
force-pushed
the
gh-actions-refactoring
branch
3 times, most recently
from
923606c to
f8b538a
Compare
Sushisource
force-pushed
the
gh-actions-refactoring
branch
from
f8b538a to
43d4fd0
Compare
|
All SDKs are able to successfully run these workflows now. temporalio/sdk-go#961 fails b/c there is a compat break, we still have to decide our exact strategy there. |
| @@ -81,31 +81,26 @@ jobs: | |||
| feature-tests-ts: | |||
| uses: ./.github/workflows/typescript.yaml | |||
| with: | |||
| typescript-repo-path: temporalio/sdk-typescript | |||
| typescript-repo-ref: main | |||
| version: 1.4.4 | |||
There was a problem hiding this comment.
I have opened #166 for --version latest support. I may be able to get to it today.
I wonder, even if we add that, would it be ok to put in here? Or are there concerns of the mismatch in time between when an SDK is released and when SDK features is updated to support that release? If we're not gonna put it in here, I might not even work on the issue as it may not be worth it.
There was a problem hiding this comment.
Yeah, that's a good question. I kinda like the explicitness of this.
We could maybe do something like check if the explicit version is not latest and let us know somehow? I'm just not sure what the delivery mechanism for that would be.
There was a problem hiding this comment.
Pros of explicitness:
- No downtime for SDK features because it has to be explicitly upgraded instead of being different
Cons of explicitness:
- If a release doesn't break any SDK features, people will have no reason to update this and it may get stale
Yeah, there's no good notification mechanism that can't be ignored, heh.
Maybe at the least we should pull the version out of what is set in the SDK features repo root so we don't have to remember to update there and here? Something like --version current? Also then you could put dependabot on this repo to tell you when to upgrade (surely you can configure it to just be for our SDKs and not all of its other noise).
Would be curious @bergundy's opinion here.
There was a problem hiding this comment.
+1 for #166, I'd just make sure that we get the concrete version when we build docker containers so we can properly tag the images.
There was a problem hiding this comment.
I'd be fine with explicit version too as long as we add a release step in our guidelines to bump here or better yet if there was a good way to automate it (assuming we have time for it).
| @@ -138,17 +124,6 @@ jobs: | |||
| semver-latest: major | |||
| run-tag: '1.4.4' | |||
|
|
|||
| # TODO: Need PR to add some gradle stuff in sdk-java to simplify this, but also we're not going | |||
There was a problem hiding this comment.
While technically we could add this trigger from Java SDK to get master docker builds pushed to a repository I don't think master images are a part of our matrix.
I would want us to test against cloud on SDK main / master but that doesn't require building a docker image.
There was a problem hiding this comment.
(and shouldn't be defined as a part of this matrix so we're good)
| @@ -0,0 +1,64 @@ | |||
| name: Java Features Testing | |||
| const errstr = `Feature ${featureDir} failed with ${err}`; | ||
| failedFeaturesStr += errstr + '\n'; | ||
| console.error(errstr, (err as any).stack); |
There was a problem hiding this comment.
This should do it.
| const errstr = `Feature ${featureDir} failed with ${err}`; | |
| failedFeaturesStr += errstr + '\n'; | |
| console.error(errstr, (err as any).stack); | |
| console.error(`Feature ${featureDir} failed`, err); |
There was a problem hiding this comment.
The point here was to include each failure in the summary at the end of the CI log where people will see it
Indeed I was considering blasting those, but they run linters which aren't necessary in the reusable workflow defs. I could write ifs to exclude them or just run them every time... but the isolation maybe makes things a touch more readable. 🤷♂️ We can always get rid of them later if it's a problem. |
What was changed
Why?
Great testathon 2022