search

SeaQL / sea-orm

🐚 An async & dynamic ORM for Rust
https://www.sea-ql.org/SeaORM/
Apache License 2.0
6.56k stars 459 forks source link

Add `ActiveValue::try_as_ref()` #2197

Closed Expurple closed 1 month ago

Expurple commented 2 months ago

Another helper method that I use very often and would like to upstream. Basically, a non-panicking version of ActiveValue::as_ref.

PR Info

New Features

Bug Fixes

Breaking Changes

Changes

tyt2y3 commented 2 months ago

Thanks. But is try_as_ref the best name? I am trying to find something similar in other Rust libraries.

Expurple commented 2 months ago

I think, it's a good name because it's consistent with the existing as_ref, and there's also a third party try_as_traits::TryAsRef::try_as_ref for using this pattern generically. I've decided against adding an extra dependency because I don't need to be abstract & generic, and inherent methods are more discoverable.

In my dependent codebase, this helper is called ActiveValueExt::value. But when I was upstreaming it, I discovered that this name would be confusing in combination with existing into_value and into_wrapped_value which return Value rather than V.

Overall, it's up to you. I'm ok with other names, as long as they don't contain value. E.g. as_ref (if it wasn't already taken), borrow (you may not like it because it sounds like Borrow::borrow but has a different signature), get...

For other examples in the ecosystem, you can check out the methods on async_graphql::MaybeUndefined. It's a similar Option-like enum

billy1624 commented 2 months ago

Hey @Expurple, thanks for contributing!!

Maybe value or as_value? We already have unwrap, into_value and into_wrapped_value methods.

https://github.com/SeaQL/sea-orm/blob/c59e28f8c0319accc7177d3c37fe7bd2a8892b82/src/entity/active_model.rs#L815-L842

Expurple commented 2 months ago

Just choose any one name that you agree to merge, then I make the edit and you merge it. The final approve decision is up to you anyway. I've already listed some options and explained why I prefer try_as_ref/borrow/get to value/as_value, but all of these will do the job and I'm not interested in bikeshedding

billy1624 commented 1 month ago

Hey @tyt2y3, perhaps value / as_value?

github-actions[bot] commented 1 month ago

:tada: Released In 1.0.0-rc.5 :tada:

Thank you everyone for the contribution! This feature is now available in the latest release. Now is a good time to upgrade! Your participation is what makes us unique; your adoption is what drives us forward. You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.