-
Notifications
You must be signed in to change notification settings - Fork 978
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
add Faraday#Deprecate
to 1.x
#1438
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for bringing this back @hyuraku.
There are some changes that I wouldn't expect in this PR, please see my comment below.
Do you also think you could add some test coverage to check the warnings are correctly output when the skip is disabled?
Thank you for addressing all the comments @hyuraku 🙌!
|
I want to add the test when the skip is disabled? to |
Or better, a generic test around the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just realised we can now remove the require
from error.rb
as well
lib/faraday/error.rb
Outdated
@@ -1,5 +1,7 @@ | |||
# frozen_string_literal: true | |||
|
|||
require 'faraday/deprecate' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not necessary anymore 🙌 !
@iMacTia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic work @hyuraku and great test coverage 💯👏
The rubocop
offence is unrelated to the PR itself, I'll test this locally and merge if all tests are passing 👍
All tests pass locally ✅ 🎉 |
Thank you both! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, mistake
please delete this message🙏
Description
add Faraday#Deprecate to hide deprecate method's warning message
Fixes #1410
Todos
List any remaining work that needs to be done, i.e:
Additional Notes