Skip to content

replace futures with futures-util#288

Merged
la10736 merged 1 commit intola10736:masterfrom
mati865:push-lzmnyxtqpsyz
Dec 5, 2024
Merged

replace futures with futures-util#288
la10736 merged 1 commit intola10736:masterfrom
mati865:push-lzmnyxtqpsyz

Conversation

@mati865
Copy link
Copy Markdown
Contributor

@mati865 mati865 commented Dec 4, 2024

This slightly reduces depedency tree.

This slightly reduces depedency tree.
Copy link
Copy Markdown
Owner

@la10736 la10736 left a comment

Choose a reason for hiding this comment

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

Seams that this change broke the msrv test 😢

We should go little bit deeper to understand what's happen and if is possible to leave the same msrv or we should bump it

@mati865
Copy link
Copy Markdown
Contributor Author

mati865 commented Dec 4, 2024

The failure is unrelated. futures crate pulls in futures-util anyway, so the only thing that changes here is using code directly instead of reexport from futures. I'm pretty sure any PR would fail like that.

I don't know this test setup, but to me it looks like toolchain in version 1.83 has generated lockfile in version 4 (this is the new default), but version 4 is supported only by toolchains 1,78+.

@la10736
Copy link
Copy Markdown
Owner

la10736 commented Dec 5, 2024

The failure is unrelated. futures crate pulls in futures-util anyway, so the only thing that changes here is using code directly instead of reexport from futures. I'm pretty sure any PR would fail like that.

I don't know this test setup, but to me it looks like toolchain in version 1.83 has generated lockfile in version 4 (this is the new default), but version 4 is supported only by toolchains 1,78+.

Maybe you're right. Yesterday it was midnight and I was too tired to check msrv on main branch: I just trusted the weakly CI... But there is something wired here: the weakly CI was run 3 days ago and didn't find any issues.

Now I run again the CI on main to check if there is something new.

@la10736
Copy link
Copy Markdown
Owner

la10736 commented Dec 5, 2024

Now I run again the CI on main to check if there is something new.

Ok, now the CI is failed also in main in the same way. I guess that a new cargo hack version was released in these days.

I can merge this PR because the issue is un related

Copy link
Copy Markdown
Owner

@la10736 la10736 left a comment

Choose a reason for hiding this comment

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

Thanks

LGTM

@la10736 la10736 merged commit 9a78bfb into la10736:master Dec 5, 2024
@mati865 mati865 deleted the push-lzmnyxtqpsyz branch December 5, 2024 17:14
@mati865
Copy link
Copy Markdown
Contributor Author

mati865 commented Dec 5, 2024

No worries, thanks for the quick response.

@mati865
Copy link
Copy Markdown
Contributor Author

mati865 commented Dec 5, 2024

I don't know this test setup, but to me it looks like toolchain in version 1.83 has generated lockfile in version 4 (this is the new default), but version 4 is supported only by toolchains 1,78+.

I've made a quick look at I'm certain this is the case. Pinning the toolchain in MSRV job to anything lower than 1.83 will avoid the problem.

@la10736
Copy link
Copy Markdown
Owner

la10736 commented Dec 5, 2024 via email

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.

2 participants