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

Allow setting logging callback. #54

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

jevolk
Copy link
Contributor

@jevolk jevolk commented Apr 15, 2024

This will need facebook/rocksdb#12537
Screenshot from 2024-04-15 00-24-16

Signed-off-by: Jason Volk <jason@zemos.net>
@jevolk
Copy link
Contributor Author

jevolk commented May 21, 2024

std::ptr::from_ref (1.76) slightly exceeds the MSRV (1.75). Would you like me to downgrade that or are you interested in bumping?

@zaidoon1
Copy link
Owner

sorry, missed the notification for this for some reason, initially I wanted to stay with the latest, however, it looks like this is not recommended as it may cause issues for users of this crate. I'll likely adopt a similar policy to tokio. So let's not try to upgrade the MSRV for now.

@girlbossceo
Copy link

Do you know any specific users you'd like to reach out to for permission to bump the MSRV in your fork of rust-rocksdb? conduwuit would really like this feature, our MSRV is already the latest.

@zaidoon1
Copy link
Owner

zaidoon1 commented May 26, 2024

would really like this feature

sorry, maybe i'm misunderstanding, the std::ptr::from_ref requires the new version, but you don't care about this part right? You can do it a different way (based on the comment Would you like me to downgrade that) and still get this pr/feature merged

@girlbossceo
Copy link

Yes just using an as conversion is fine, but I'm just wondering if an exception for a MSRV raise could be done here. If not, that's fine and you're right this feature will still land either way.

@zaidoon1
Copy link
Owner

we can bump the MSRV one more time before we restrict it to the 6 month rolling releases.

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.

None yet

3 participants