-
Notifications
You must be signed in to change notification settings - Fork 136
Add Node 24 to CI test matrix #1762
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
base: main
Are you sure you want to change the base?
Conversation
c535787
to
5dbbedf
Compare
5dbbedf
to
c85562f
Compare
"node_modules/nexus-rpc": { | ||
"version": "0.0.1", | ||
"resolved": "git+ssh://git@github.com/nexus-rpc/sdk-typescript.git#f594a7fd9e33bd14e5ce1ed04c5225fc708e7866", | ||
"resolved": "https://registry.npmjs.org/nexus-rpc/-/nexus-rpc-0.0.1.tgz", |
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.
Switching from the git reference to the published package over the git one as NPM 11 has a breaking change which skips prepare
scripts from running when --ignore-scripts
is used. nexus-rpc
needs prepare
to produce lib/
files which are not checked in, but are present in the published package.
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.
Uhh? Actually the git-based dependency should have never been commited to main, not sure how come it slipped through. Replacing it with the published release is definitely the right thing to do.
1673830
to
c87ea5f
Compare
dedent` | ||
ApplicationFailure: Fail me | ||
at Function.nonRetryable (common/src/failure.ts) | ||
at $CLASS.nonRetryable (common/src/failure.ts) |
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.
That comes out very neat. Great!
'Error' | ||
), | ||
], | ||
[makeFailWorkflowExecution('Signal failed', 'Error')], |
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.
What if we kept the reference stack trace as argument to makeFailWorkflowExecution()
, and instead did the special handling of stack traces as part of compareCompletion()
? We'd simply iterate over history events, looking for WorkflowExecutionFailure
events (if any). WDYT?
What was changed