search

ruby-grape / grape-entity

An API focused facade that sits on top of an object model.
MIT License
721 stars 153 forks source link

🔥 Remove `FetchableObject` #369

Closed danielvdao closed 1 year ago

danielvdao commented 2 years ago

Context

A PR addressing this issue: https://github.com/ruby-grape/grape-entity/issues/368

The thing I am confused about is why we check if object.respond_to?(:fetch, true). Implementing #fetch is tricky and I think that most of the time (if not always) we should rely on Hash to implement #fetch as that's in Ruby stdlib. Otherwise just default to #public_send.

Happy to close if it doesn't make sense either - but wanted to give it a shot and it looks like all tests still pass.

cc @LeFnord

LeFnord commented 1 year ago

sorry @danielvdao … for the latency, shame on me

please can an CHANGELOG and UPGRADE entry thanks

danielvdao commented 1 year ago

Thanks, I'll take care of it 🔥

danielvdao commented 1 year ago

@LeFnord done! Let me know if this is what you were looking for.

danielvdao commented 1 year ago

Hey @LeFnord some of the Rubocop violations are going off, but I don't think they're related to my PR. What do you think? 🤔

LeFnord commented 1 year ago

mmh, will have an eye on

LeFnord commented 1 year ago

I'm on it

LeFnord commented 1 year ago

thanks again