meilisearch / heed

A fully typed LMDB wrapper with minimum overhead 🐦

https://docs.rs/heed

MIT License

519 stars 52 forks source link

replace all `FnMut` with `FnOnce` in `put_reserve_*` #251

Closed nolanderc closed 3 months ago

nolanderc commented 3 months ago

Pull Request

Related issue

N/A

What does this PR do?

Replaces FnMut with FnOnce in all put_reserve_* functions.
- A value should only be written once, so FnMut is needlessly restrictive; with FnOnce we allow more functions to be used.

PR checklist

Please check if your PR fulfills the following requirements:

[x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
[ ] Have you read the contributing guidelines?
[x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Kerollmops commented 3 months ago

I am wondering if this is a breaking change 🤔 Because it is not constraining more....

nolanderc commented 3 months ago

I am wondering if this is a breaking change 🤔 Because it is not constraining more....

It shouldn't be: any code which previously passed a FnMut closure will still compile as all FnMut closures also implement FnOnce.

Granted, this is a signature change, so if somebody passed the function pointer to another function foo(Database::put_reserved), where foo had an FnMut bound on the write_value parameter, this would no longer compile. So technically this is breaking, but I can’t think of why you would do something like that…

Kerollmops commented 3 months ago

Ok, so we can call that a soft-breaking change, and therefore, it's allowed 🤭 Thank you. It will be released in v0.20.1, then.