Skip to content
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

6000 reduce floating promises #9416

Merged
merged 15 commits into from
May 28, 2024
Merged

6000 reduce floating promises #9416

merged 15 commits into from
May 28, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented May 28, 2024

refs: #6000

Description

I keep seeing these warnings and they're distracting. This burns down almost all of them. What's left are a handful in Zoe that may be best for @Chris-Hibbert to tackle directly.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

Some of these change the test. Reviewers should be confident in the changes.

Upgrade Considerations

None of these should change the end-user API. Some may fix bugs.

@turadg turadg requested review from iomekam and Chris-Hibbert May 28, 2024 18:15
Copy link

cloudflare-workers-and-pages bot commented May 28, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: e332a3a
Status: ✅  Deploy successful!
Preview URL: https://45e5b670.agoric-sdk.pages.dev
Branch Preview URL: https://6000-reduce-floating-promise.agoric-sdk.pages.dev

View logs

Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One correction. The rest looks good.

@@ -78,7 +77,7 @@ test('tick does not flush by default', async t => {
const handler = Far('handler', {
wake: scheduled => {
woken = scheduled;
stallLots().then(() => (later = true));
void stallLots().then(() => (later = true));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be await. stallLots() is just a delay. it does nothing with void.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I punted some Zoe already I just punted the rest of Zoe to https://github.com/Agoric/agoric-sdk/compare/6000-no-zoe-floating-promises

(I lost one of my commits, unfortunately, but that has the Zoe one from this branch.)

@turadg turadg force-pushed the 6000-reduce-floating-promises branch from 8c54d65 to 98614ce Compare May 28, 2024 19:02
@turadg turadg marked this pull request as ready for review May 28, 2024 19:09
@turadg turadg requested a review from Chris-Hibbert May 28, 2024 20:10
@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label May 28, 2024
@turadg turadg force-pushed the 6000-reduce-floating-promises branch from 98614ce to e332a3a Compare May 28, 2024 20:17
@mergify mergify bot merged commit bb3d92f into master May 28, 2024
63 checks passed
@mergify mergify bot deleted the 6000-reduce-floating-promises branch May 28, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants