Support Faraday 2.x on 1-4-stable branch, for subsequent 1.x release

[jrochkind]

For subsequent 1.x release supporting faraday 2.x -- gemspec is unchanged as to how old a faraday it allows, all the way back to 0.8 as per previous 1.x release.

Backported enough from master to get tests passing on both/either faraday 1.x AND 2.x.

I have confirmed locally that bundle exec rspec passes using both Faraday 1.9.3 and Faraday 2.2.0, by making local uncommitted edits the Gemfile to force both.

CI will of course only run with one version of Faraday. I'm not sure if it will actually be 1.x or 2.x -- see #561 (comment)

But this should make it possible to do a 1.4 release that allows and works with Faraday 2.x?

[pboling]

@jrochkind this looks great! Please run rubocop and fix lint issues until it is clean.

I will handle the failure in the 2.5 unit tests workflow (build error, need to install latest rubygems/bundler), and the Danger error.

[pboling]

@jrochkind if you rebase on 1-4-stable at HEAD, I have all but the build for ruby-head passing. Working on that now.

[jrochkind]

Thanks! Oh I forgot to run rubocop locally first (I did run rspec locally!) -- weird I totally just copied code from master, surprising that rubocop can handle it in master but not here, but ok I'll deal with it tommorow!

[jrochkind]

Note that CI log confirms that CI is unfortunately running with faraday 1.9.3 not 2.x (Using faraday 1.9.3) -- this is true on master too at the moment though, it's the same. Due to #561 (comment)

[pboling]

1-4-stable and master leap frog each other depending on which one I worked on last. ;)

I will try to get it setup so we can test both versions of Faraday with discrete gemfiles.

[jrochkind]

At the moment danger is incompatible with faraday 2 -- since danger is only used in the Gemfile, and only under certain circumstances, and not the gemspec, this does not keep an actual runtime using oauth2 from using faraday2. But it does keep us from testing with faraday 2 (including in master branch, this is not unique to this branch, but was already true in master branch). I filed an issue and tried to PR to danger, but then ran into an issue where one of it's dependendencies won't allow faraday2... filed an issue there too.

This faraday upgrade really has created kind of a dependency challenge.

I guess there is already a conditional in the Gemfile around the danger dependency, I guess you could alter it so in the faraday 2 CI run danger is not included? I guess that would work!

[pboling]

Yeah, that sounds like a descent solution. Will do that.

[jrochkind]

Sorry I'm taking so much of your time, I appreciate it, I know you'd rather be focused on oauth2 2.0!

For some reason it needs you to approve the CI workflow again? From my local rubocop run:

Rubocop wants me to turn this:

          if options[:connection_build]
            options[:connection_build].call(builder)
          end

Into:

          options[:connection_build]&.call(builder)

Really, you want me to do that?

(deleted other question, I'm getting to the bottom of it)

[pboling]

No, let's not make that change.

In fact, we need to make the opposite-ish change in the Rubocop rule that is asking you to do that.

The problem we have with Rubocop, and ostensibly supporting very old Rubies still on 1.4.x (and we can't drop them without a major version bump) is that latest Rubocop doesn't support prior to Ruby 2.5, so the style suggestions are using Ruby features we can't ever adopt on the 1-4-stable branch. What rule was it, and can you disable it, or instruct the reverse, in the .rubocop.yml?

[pboling]

Up until yesterday we still had all the old AF rubies still "running" on Travis CI, but I removed the config yesterday because old Travis is pretty dead (I.e. I don't think it was actually running any new jobs), and I don't know how to migrate to new Travis.

[jrochkind]

OK I'll figure out how to at least make an exception for this line. (I'm not a rubocop expert, I tend to fight with it)

(It was a combo of two rules -- use a one-line if for a one-line body, and use safe navigation instead of a condition for hash value presence -- not totally sure if you want to change it globally, I'm just going to focus on this PR)

Travis basically no longer provides free service for open source, although they aren't very clear about this and sometimes pretend otherwise. But IMO it's not worth trying to migrate to anything on "new travis" unless you are expecting to pay for it. But I see you are already on Github Actions, which is what I've moved all my stuff to also and works out well.

[pboling]

LOL, just realized I can see the rubocop failure in the build. I'll setup the rule now.

[pboling]

@jrochkind The rule is now disabled in the 1-4-stable branch. If you rebase you won't need to do anything else.

[jrochkind]

I didn't rebase yet, but looks like you merged anyway! Thank you so much!

Thoughts on timeline for a 1.4.x release? (I'm not in a huge hurry, but hope it won't get forgotten forever either!)

[pboling]

@jrochkind sorry I failed to respond here, but 1.4.9 is out!