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

Bump faraday dep version from >= 0.9.2 to >= 2 and migrate custom middleware. Fixes ylecuyer/survey-gizmo-ruby#121 #122

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nicduke38degrees
Copy link

Followed the faraday ~2 update docs here
Also used referenced a some legacy and current faraday source code to guide me in the migration of the custom ParseSurveyGizmo middleware

The new middleware template offered for faraday 2, contains some useful code comments
lostisland/faraday-middleware-template/blob/main/template/lib/gem_path/middleware.rb.erb

The legacy ResponseMiddleware class
lostisland/faraday_middleware/blob/main/lib/faraday_middleware/response_middleware.rb

The new implementation of JSON Response Middleware that is bundled with faraday 2
/lostisland/faraday/blob/main/lib/faraday/response/json.rb

Tests that cover the custom parser are passing. Wasn't sure if there was a need for adding more. I'm fairly new to Ruby to any pointer welcomed and I'm happy to add some specs given a bit of direction 🙏

@nicduke38degrees nicduke38degrees changed the title Fixes #121. Bump faraday dep version from >= 0.9.2 to >= 2 and migrate custom middleware Bump faraday dep version from >= 0.9.2 to >= 2 and migrate custom middleware. Fixes ylecuyer/survey-gizmo-ruby#121 Aug 3, 2023
@ylecuyer
Copy link
Owner

ylecuyer commented Aug 5, 2023

Thanks for the PR ! sadly I have no more access to survey gizmo (aka alchemer) API to properly test it.

@peret @dgimb89 do you want to give it a try? I saw you work on the playtestcloud fork

@@ -18,8 +18,7 @@ Gem::Specification.new do |gem|
gem.add_dependency 'activesupport', '>= 3.0'
gem.add_dependency 'addressable', '>= 2'
gem.add_dependency 'awesome_print', '>= 1'
gem.add_dependency 'faraday', '>= 0.9.1'
gem.add_dependency 'faraday_middleware'
gem.add_dependency 'faraday', '>= 2.0'
Copy link
Owner

Choose a reason for hiding this comment

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

could you remove ruby 2.4 and 2.5 from CI as faraday >2 requires at least tuby 2.6 and any way those version are already EOL

Copy link
Author

@nicduke38degrees nicduke38degrees Aug 7, 2023

Choose a reason for hiding this comment

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

Removed 2.4 and 2.5
Also updated gemspec to set min ruby version to >=2.6
Awaiting maintainer to run the Test workflow

All <3 Ruby is EOL, would you like further adjustments to remove all 2.x versions?

@nicduke38degrees
Copy link
Author

@ylecuyer @peret @dgimb89
is anyone able to rereview and progress this fix please?

@dgimb89
Copy link

dgimb89 commented Aug 16, 2023

@ylecuyer @peret @dgimb89 is anyone able to rereview and progress this fix please?

I will have a look on Saturday

@nicduke38degrees
Copy link
Author

@ylecuyer @peret @dgimb89 is anyone able to rereview and progress this fix please?

I will have a look on Saturday

@dgimb89 thanks for the reply on this. Have you had a chance to take a look?

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.

3 participants