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

Add support for faraday 2.x #412

Merged
merged 8 commits into from
May 2, 2024
Merged

Conversation

lavoiesl
Copy link
Contributor

@lavoiesl lavoiesl commented Nov 25, 2023

Includes a rebased version of #402

Fixes #407

  • Prefix activesupport appraisals, as to not conflict with faraday appraisals
  • Bump the minimum Faraday requirement to 1.10 to pick up json support: Add json middleware to 1.x lostisland/faraday#1400
  • Allow Faraday 2.x, it doesn't require any more change than using 1.10's json support
  • Remove the obsolete ParseJson middleware
  • Add Faraday 1.10 and 2.x appraisals

Notice how no tests have changed

@sarahsehr
Copy link

@gaorlov Bump please :) Faraday 2.0 has been out for almost 2 years.

@timbogit
Copy link

timbogit commented Apr 1, 2024

@gaorlov Is this still being considered? My team depends heavily json_api_client as well as faraday, so we would really appreciate this being merged. 🙇

@dolan
Copy link

dolan commented Apr 30, 2024

bumping again @gaorlov same situation here as @timbogit

@gaorlov
Copy link
Collaborator

gaorlov commented May 2, 2024

Hi, folks!
Sorry about the delayed response and thank you for the contributions!
Quick question: what is the thinking behind removing gemfiles 5.2 and 6.0?

@lavoiesl
Copy link
Contributor Author

lavoiesl commented May 2, 2024

I don't remember making a conscious decision to remove those. Looking at the commit, it looks live it was auto-removed when running the appraisal generator.

Is it possible that those are obsolete files?

@gaorlov
Copy link
Collaborator

gaorlov commented May 2, 2024

i must admit i am not up to date on rails versions. It looks like 6.0 has reached EOL, but 6.1 is still supported. Can we bring it back and just test against 6 instead of a specific sub-version? i don't have a strong opinion on 5.2.3

@gaorlov
Copy link
Collaborator

gaorlov commented May 2, 2024

Looking at the diff again, it does seem like an outdated file. It doesn't actually require rails, just activesupport

is there another mechanism by which we test that activesupport version?

also: looks like there's a bunch of old ruby failures. i think it's probably okay to drop ruby 2.3, 2.4, and 2.5 from the test matrix if you want to drop them from the workflow

@lavoiesl lavoiesl force-pushed the seb-faraday-2.x branch 2 times, most recently from 5c875cf to 1eb9bc9 Compare May 2, 2024 20:06
@lavoiesl
Copy link
Contributor Author

lavoiesl commented May 2, 2024

New matrix:

  • Ruby: 3.0, 3.1, 3.2, 3.3
  • Activesupport: 6.0, 6.1, 7.0, 7.1
  • Faraday: 1.x, 2.x

Updated the changelog as well

- ruby: "2.7"
rails: "4.2"
ruby:
- "3.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needs to be quoted, otherwise 3.0 becomes 3, which gets interpreted as 3.x, resolving to 3.3

@gaorlov
Copy link
Collaborator

gaorlov commented May 2, 2024

marvelous. thank you for pulling in appraisals! that's a really nice tool

@gaorlov gaorlov merged commit d4c421c into JsonApiClient:master May 2, 2024
4 checks passed
@lavoiesl
Copy link
Contributor Author

lavoiesl commented May 2, 2024

Oddly enough, it was there as a dependency and the configurations were there, but it wasn't being used in the tests 🤷🏼‍♂️

@gaorlov
Copy link
Collaborator

gaorlov commented May 2, 2024

thank you for you contributions and patience!
1.23.0 is now live with your changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Faraday > 2.x
6 participants