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

Disallow futures::executor::block_on #10912

Merged
merged 6 commits into from
Apr 18, 2023
Merged

Disallow futures::executor::block_on #10912

merged 6 commits into from
Apr 18, 2023

Conversation

oxade
Copy link
Contributor

@oxade oxade commented Apr 14, 2023

Description

  • Adds clippy warning wherever futures::executor::block_on is used
  • Area owners should address. Looks like mostly in indexer, narwhal, sui-json, sui-json-rpc, sui-core

For more context see: #10910

For now we add exceptions on some functions while the area owners fix the issues

Test Plan

N/A


If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

@vercel
Copy link

vercel bot commented Apr 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2023 8:15pm
explorer-storybook ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2023 8:15pm
sui-wallet-kit ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2023 8:15pm
wallet-adapter ⬜️ Ignored (Inspect) Visit Preview Apr 18, 2023 8:15pm

@andll
Copy link
Contributor

andll commented Apr 14, 2023

We should probably wait until the usages in codebase are removed before merging this

@oxade
Copy link
Contributor Author

oxade commented Apr 14, 2023

We should probably wait until the usages in codebase are removed before merging this

Thats the plan. It wont merge otherwise cos clippy is failing

@oxade oxade requested a review from joyqvq as a code owner April 18, 2023 03:55
@vercel vercel bot temporarily deployed to Preview – wallet-adapter April 18, 2023 03:55 Inactive
@vercel vercel bot temporarily deployed to Preview – explorer-storybook April 18, 2023 03:56 Inactive
@vercel vercel bot temporarily deployed to Preview – sui-wallet-kit April 18, 2023 03:56 Inactive
@oxade oxade marked this pull request as draft April 18, 2023 03:56
@oxade oxade force-pushed the disallow_future_block_on branch from 884199b to 0b2b0f9 Compare April 18, 2023 08:28
@oxade oxade marked this pull request as ready for review April 18, 2023 18:08
@vercel vercel bot temporarily deployed to Preview – sui-wallet-kit April 18, 2023 18:08 Inactive
@vercel vercel bot temporarily deployed to Preview – explorer-storybook April 18, 2023 18:09 Inactive
@oxade oxade enabled auto-merge (squash) April 18, 2023 20:15
@oxade oxade merged commit f53b960 into main Apr 18, 2023
@oxade oxade deleted the disallow_future_block_on branch April 18, 2023 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants