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

Bump libloading to 0.8 #739

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Bump libloading to 0.8 #739

merged 1 commit into from
Nov 15, 2023

Conversation

djkoloski
Copy link
Contributor

No description provided.

@MarijnS95
Copy link
Collaborator

MarijnS95 commented Apr 11, 2023

That's pretty rapid, the update is only out for 3 hours 😅... Any comment on what's new in this semver-breaking release, why we need it, and if it needs to be backported to the latest stable release?

@djkoloski
Copy link
Contributor Author

Oh! Sorry, I didn't mean to make it seem like this was urgent. 😅 I just happened to be in the right place at just the right time to notice this bump.

The only major change I'm aware of is that libloading switched from winapi to windows-sys, which I'm excited for since it slims down my dependencies. I figured it was a quick change to make and wanted to be helpful. 🙂

@MarijnS95
Copy link
Collaborator

Okay thanks, ditching winapi is definitely worth upgrading for. I'll do some extra scanning of the changelog to make sure this doesn't change any other semantics. However, as libloading is technically part of our public API in LoadingError::LibraryLoadFailure we cannot backport this to the current stable release.

The next breaking ash release is something I'd like to slowly start creeping towards though as we've accumulated quite a few useful breaking features now, but it is also right around this time that a lot of technically breaking changes are flowing in and I'd rather see that stabilize first.

@Ralith
Copy link
Collaborator

Ralith commented Apr 11, 2023

libloading switched from winapi to windows-sys

Hmm, isn't the latest guidance to use windows-bindgen?

However, as libloading is technically part of our public API in LoadingError::LibraryLoadFailure

Perhaps we should fix this.

@MarijnS95
Copy link
Collaborator

@Ralith windows-bindgen is the way to go, yes, but it's fairly new and I hope libloading can migrate to that without yet another breaking bump. Regardless, I've asked about it at nagisa/rust_libloading#120 (comment) though Jake might perform this conversion soon enough.

I'm fine dropping libloading from the public thiserror-like enum fwiw, but can only do that on main again.

@AngelicosPhosphoros
Copy link

@MarijnS95 If you closed my PR for this, when this PR would be merged? It's been half a year already.

@MarijnS95 MarijnS95 merged commit d40ab4b into ash-rs:master Nov 15, 2023
@MarijnS95
Copy link
Collaborator

MarijnS95 commented Nov 15, 2023

@MarijnS95 If you closed my PR for this

Let's not have duplicate PRs open tackling the same thing while diverging conversations. Before opening a PR, consider searching whether the same thing is already open, and if so seek if you can help further that effort instead.

when this PR would be merged? It's been half a year already.

@AngelicosPhosphoros it's been merged now. Conflicts just had to be resolved, and we have an ongoing discussion about libloading in the public API.

@AngelicosPhosphoros
Copy link

Hooray!

@MarijnS95 MarijnS95 added this to the Ash 0.38 milestone Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants