[BUG]: The type of the dependency on faraday-retry is irritating and unclear.
expeehaa opened this issue · 2 comments
What happened?
When using the gem with faraday > 2
without having faraday-retry
installed, octokit.rb
always prints this warning message.
To use retry middleware with Faraday v2.0+, install `faraday-retry` gem
According to #1486, there are currently 2 ways to disable the warning:
- disable all warnings by setting
OCTOKIT_SILENT=true
- install
faraday-retry
The first option has the side effect of disabling any warning from octokit.rb
, which is clearly not my intention. The second option requires me to install a gem, but this appears to actually be kind of optional. I had no issues yet without faraday-retry
and usages test if its constants are defined:
octokit.rb/lib/octokit/default.rb
Lines 36 to 42 in fb01ac2
In case I do not want to use
faraday-retry
for whatever reason, I will therefore always be left with that annoying warning.
From this perspective, it is not clear what kind of dependency octokit.rb
actually has on faraday-retry
. It is not required, cannot be listed in the gemspec since faraday ~> 1.0
is also supported, but also is not fully optional since the exclusion affects all programs using octokit.rb
by printing a warning.
If it is optional, I would expect to at most get a post-install message of the warning (probably rather a README entry), but not a forced warning. If it is required, I would expect require 'octokit.rb'
to raise the warning message as an error when require 'faraday/retry'
raises a LoadError
.
Implementing either of those options seems quite trivial, contrary to what was claimed in #1486 in this comment. Did I miss something there?
I’ld happily create a PR, although I have no idea which options would be preferred, if any.
Versions
octokit.rb = 6.1.1
, faraday = 2.7.4
without faraday-retry
Relevant log output
No response
Code of Conduct
- I agree to follow this project's Code of Conduct
👋 Hey Friends, this issue has been automatically marked as stale
because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned
label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!
I did not test this, but I suppose it would be possible to create an empty meta gem (e.g. octokit-faraday-dependencies
) that has exactly two versions:
- Version
1.0
requiresfaraday >=1, < 2
. - Version
1.1
requiresfaraday >=2, <3
and additionallyfaraday-retry
(andfaraday-multipart
, if necessary).
octokit
itself would then requireoctokit-faraday-dependencies ~> 1.0
. By default, the newestfaraday
2.x should then be used, unless some other dependency specifiesfaraday < 2
, which would resolve the meta gem to version1.0
.
I don’t really like this solution since it introduces a new gem, but I guess it would work. Unless gems start allowing dependency-version-conditional additional dependencies, I don’t really see any way other than that, requiring faraday >= 2
or the above options (which seem to have been rejected in #1569).