-
Notifications
You must be signed in to change notification settings - Fork 291
Create a dedicated failure type for AsyncCancelled #5988
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
|
|
||
| asyncCancelledFailure = | ||
| DataDeclaration | ||
| (Unique "f9fdcae7d12e37728dc7336097d4255c1277f1c7a46c6200e9d3d57b8d8d7273") |
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 just grabbed 32 bytes from /dev/random and converted them to hex. Is there something else I should have done?
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 forget how the others were made. Maybe I was using uuidgen | sha256sum. In any case, random bytes should be okay, too.
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.
IIRC, it currently expects it to be a valid unison identifier? so not every hex string will work, but this one does.
dolio
left a comment
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 fine to me.
|
Oh yeah, you need to run the transcripts locally and commit changes. |
f2881a7 to
dccde58
Compare
This is similar to the behavior for `ThreadKilledFailure`. The idea is that inspecting for this is more stable than looking for `"AsyncCancelled"` in the message part of the `Failure`. I don't really know what I'm doing with adding builtins so I probably missed something.
dccde58 to
795228c
Compare
|
@ceedubs maybe bad in principle, but it turns out probably better to just make a ceedubs/ branch on the main repo, vs pr from your fork, because i'm limited in being able to run quick fix gha on your fork i tried to do it manually from my home computer, but i ran into my nix signing issue again and couldn't complete the fixup as a result |
This is similar to the behavior for
ThreadKilledFailure. The idea is that inspecting for this is more stable thanlooking for
"AsyncCancelled"in the message part of theFailure.