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

fix: agree one a single default runtime for the whole workspace #1988

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

crepererum
Copy link
Contributor

This fixes cargo check --workspace and rust-analyzer.

Also see #1956.

@Kinrany
Copy link

Kinrany commented Jul 21, 2022

Could you explain why changing the runtime to async-std helps?

@crepererum
Copy link
Contributor Author

I'm not entirely sure what depends on what in this workspace, but it seems the members at least don't agree on the defaults:

https://github.com/launchbadge/sqlx/blob/main/sqlx-macros/Cargo.toml#L19

So the Cargo feature unification will pick both tokio and async-std for the RT crate, which then will fail to compile.

@abonander
Copy link
Collaborator

If we have to standardize on a single default runtime for the whole workspace, I'd prefer it to be Tokio.

@crepererum
Copy link
Contributor Author

If we have to standardize on a single default runtime for the whole workspace, I'd prefer it to be Tokio.

Which would mean adjusting all the examples as well (because there the de facto standard is currently async-std), for which I currently don't have the resources, esp. since compiling them requires quite some manual labor.

@abonander
Copy link
Collaborator

Yeah, most of those examples were initially authored when SQLx was exclusively for use with async-std. Since then, the async ecosystem has pretty much centered around Tokio and so it would make more sense to port the examples over as well.

For the most part, it should only require some find-and-replace operations, and you can just let CI build it for you.

@crepererum
Copy link
Contributor Author

I'll see if I find some time this week. On the first glance, the paw::main macro doesn't play nicely with tokio::main. It's probably a good idea to drop paw altogether (last release was 3y back) and while I'm on it, I would probably replace structopt with an up-to-date clap (clap version 3 incorporated structopt).

@Kinrany
Copy link

Kinrany commented Jul 25, 2022

Clap's API is not completely the same as structopt, removing paw alone may be easier.

This fixes `cargo check --workspace` and rust-analyzer.

Also see <launchbadge#1956>.
@crepererum
Copy link
Contributor Author

This is now ready to review.

@abonander
Copy link
Collaborator

Thanks for going the extra mile. I really appreciate it.

@abonander abonander merged commit 05d64fb into launchbadge:main Jul 28, 2022
@Kinrany
Copy link

Kinrany commented Jul 28, 2022

As a mere user I appreciate it too! ❤️

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.

3 participants