Skip to content
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

Allow Faraday v2 usage #1411

Merged
merged 3 commits into from
May 26, 2022
Merged

Conversation

skryukov
Copy link
Contributor

@skryukov skryukov commented Apr 8, 2022

To support Faraday v2 this PR:

  • drops support for Faraday ~> 0.16.0 which shouldn't be a big deal since it was a reverted release: v0.17.0 looks to be a bad release lostisland/faraday#1049 (versions < 0.16.0 and ~> 0.17.0 are already not supported by octokit)
  • updates sawyer form ~> 0.8.0 to ~> 0.9.0
  • adds support for the changed Faraday v2 interfaces
  • conditionally includes retry middleware for Faraday v2 if faraday-retry is present and warns user if gem is not installed
  • removes Faraday < 0.9 leftovers

This PR is a successor to #1358

@akarsh-tenjin
Copy link

octokit was returning 404 not found issue and this issue is been surfacing for a week. I updated faraday in other dependencies of a project. Which did resolve the issue. as i find faraday is being used in octokit.
I assume the PR will resolve the 404 not found issue.
which is similar to #1409

@tisba please review this PR when you have time.

Thank you in advance.

Copy link

@tisba tisba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. It's a nice and minimal set to support v1 and v2 at the same time.

Nice work! :)

octokit.gemspec Outdated Show resolved Hide resolved
module Octokit
module Response
# In Faraday 2.x, Faraday::Response::Middleware was removed
BaseMiddleware = defined?(Faraday::Response::Middleware) ? Faraday::Response::Middleware : Faraday::Middleware
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting that Faraday::Middleware can be used as far back as v1.2.0: lostisland/faraday@d56742a

Comment on lines +7 to +13
if Gem::Version.new(Faraday::VERSION) >= Gem::Version.new('2.0')
begin
require 'faraday/retry'
rescue LoadError
Octokit::Warnable.octokit_warn 'To use retry middleware with Faraday v2.0+, install `faraday-retry` gem'
end
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is faraday-retry usable on Faraday 1.x? I wonder if it makes sense to just add it as a dependency if so.

Co-authored-by: Bobby McDonald <BobbyMcWho@users.noreply.github.com>
@nickfloyd nickfloyd merged commit 18785c2 into octokit:4-stable May 26, 2022
@skryukov skryukov deleted the allow-faraday-v2 branch May 26, 2022 16:26
@nickfloyd nickfloyd added Type: Feature New feature or request and removed improvement labels Oct 28, 2022
dentarg added a commit to Starkast/wikimum that referenced this pull request Jul 10, 2023
dentarg added a commit to Starkast/wikimum that referenced this pull request Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants