search

lostisland / faraday

Simple, but flexible HTTP client library, with support for multiple backends.
https://lostisland.github.io/faraday
MIT License
5.72k stars 972 forks source link

add `Faraday#Deprecate` to `1.x` #1438

Closed hyuraku closed 2 years ago

hyuraku commented 2 years ago

Description

add Faraday#Deprecate to hide deprecate method's warning message

Fixes https://github.com/lostisland/faraday/issues/1410

Todos

List any remaining work that needs to be done, i.e:

Additional Notes

iMacTia commented 2 years ago

Thank you for addressing all the comments @hyuraku 🙌! Are you planning to add unit tests as well as I mentioned above?

Do you also think you could add some test coverage to check the warnings are correctly output when the skip is disabled?

hyuraku commented 2 years ago

@iMacTia

check the warnings are correctly output when the skip is disabled?

I want to add the test when the skip is disabled? to spec/faraday/connection_spec.rb.

iMacTia commented 2 years ago

Or better, a generic test around the Faraday::Deprecate#deprecate method 👍! If we know that works as expected (e.g. with an anonymous class defined in the specs), we don't need to test it everywhere we use it

hyuraku commented 2 years ago

@iMacTia I added the spec for Faraday::Deprecate#deprecate; could you review this PR?

iMacTia commented 2 years ago

All tests pass locally ✅ 🎉

kyrofa commented 2 years ago

Thank you both!