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

[Ruby 2.7] Allow Faraday 1.0.0 and Faraday-middleware 1.0.0 with Ruby 2.7 fixes, fix base Ruby 2.3 tests #16399

Merged
merged 4 commits into from
Apr 30, 2020

Conversation

Kaspik
Copy link
Contributor

@Kaspik Kaspik commented Apr 29, 2020

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Description

Allow Faraday 1.0.0 as first final stable release with Ruby 2.7 fixes included. We still allow older version for users with older Ruby.

Testing

  • update Execute modules load up tests to Ruby 2.4 (should be higher?)
  • remove tests for Ruby 2.3 - there should be support for Ruby 2.0 for example but there are no tests for this. All PRs are failing on this and I think we should drop version 2.3 completely and keep supporting only Ruby maintained versions that are not at EOL - https://www.ruby-lang.org/en/downloads/branches/

@Kaspik
Copy link
Contributor Author

Kaspik commented Apr 29, 2020

@joshdholtz

From my point of view, we should drop Ruby 2.3 (at least, and of course Ruby 2.0 / 2.1 / 2.2) as it's at the EOL - https://www.ruby-lang.org/en/downloads/branches/.

@Kaspik Kaspik changed the title [Ruby 2.7] Allow Faraday 1.0.0 and Faraday-middleware 1.0.0 with Ruby 2.7 fixes [Ruby 2.7] Allow Faraday 1.0.0 and Faraday-middleware 1.0.0 with Ruby 2.7 fixes, add Ruby 2.7 tests Apr 29, 2020
@joshdholtz
Copy link
Member

@Kaspik I’ve been thinking about this since I first saw this PR earlier this morning and I think it is time to drop support for 2.3 and lower 😊 I will get that PR going and update docs but this is a great start by dropping those tests 👌

@Kaspik
Copy link
Contributor Author

Kaspik commented Apr 29, 2020

I didn't really drop them, I just updated the XCode 9 version to Ruby 2.4 - but I think we should/could make default any of the maintained versions instead of EOL version. :)

@joshdholtz
Copy link
Member

joshdholtz commented Apr 30, 2020

@Kaspik I know you didn’t drop them but we are going to drop them now 🥳 Those 2.3 tests were actually helpful to find when dependencies started removing support for 2.3 and then we would fix them. I just want to be done with 2.3 completely now so I will create a follow up PR that does that 😇

@Kaspik
Copy link
Contributor Author

Kaspik commented Apr 30, 2020

I can do that too if you want, anyways main fix here is faraday 1.0.0 for Ruby 2.7 users like me. :)

Let me know if you want me to do the test stuff.

@joshdholtz
Copy link
Member

@Kaspik Do 2.7 tests pass locally on your machine? 2.7 tests fail on mine. I’m using Ruby 2.7.1. I’m confused on how the tests on CircleCI are passing 🤷‍♂️

@Kaspik
Copy link
Contributor Author

Kaspik commented Apr 30, 2020

Pff, check the log... it doesn’t use ruby 2.7 even tho it has the settings :(

@joshdholtz
Copy link
Member

@Kaspik Oh noooooo... I guess I’ll be looking into that today 🙃

@Kaspik
Copy link
Contributor Author

Kaspik commented Apr 30, 2020

@joshdholtz Removed 2.7 tests for now so this is good to go and unblock other PRs. :)

@Kaspik Kaspik changed the title [Ruby 2.7] Allow Faraday 1.0.0 and Faraday-middleware 1.0.0 with Ruby 2.7 fixes, add Ruby 2.7 tests [Ruby 2.7] Allow Faraday 1.0.0 and Faraday-middleware 1.0.0 with Ruby 2.7 fixes, fix base Ruby 2.3 tests Apr 30, 2020
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@joshdholtz joshdholtz merged commit ce06c21 into fastlane:master Apr 30, 2020
@Kaspik Kaspik deleted the ruby_2_7/faraday branch April 30, 2020 16:58
@fastlane-bot
Copy link

Hey @Kaspik 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

Copy link

@fastlane-bot fastlane-bot left a comment

Choose a reason for hiding this comment

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

Congratulations! 🎉 This was released as part of fastlane 2.147.0 🚀

aried3r added a commit to aried3r/fastlane that referenced this pull request Jul 10, 2020
`danger` 8 supports Ruby 2.4+, like fastlane. The range could of course
include `danger` 4 up to 8, but since it's a development dependency, it
can be as high as we want it.

The reason to update was that it was holding other dependencies back,
like `kramdown` or `faraday`.

Talking about `faraday`, ironically it had to be locked to less than 1
because of two failing test cases with `faraday` 1.x. Specifically these
two:
`rspec ./spaceship/spec/portal/enterprise_spec.rb:21`
`rspec ./spaceship/spec/portal/portal_client_spec.rb:286`

I believe at least one of them is caused by `webmock` in combination
with `faraday` and empty arrays as parameters. See also the changelog of
`webmock` for version 3.6, which supposedly fixes this. However I
haven't tried since this would entail updating `webmock`.

The `faraday` dependency was initially loosened in a PR, however to no
`Gemfile.lock` being committed, it probably remained largely unnoticed
that another dependency (`danger`) was holding it back.

https://github.com/danger/danger/blob/v8.0.2/CHANGELOG.md#800
https://github.com/bblimke/webmock/blob/v3.8.3/CHANGELOG.md#360
fastlane#16399
fastlane#8135

Further reading about committing a `Gemfile.lock` file:
https://bundler.io/blog/2018/01/17/making-gem-development-a-little-better.html
joshdholtz pushed a commit that referenced this pull request Jul 10, 2020
`danger` 8 supports Ruby 2.4+, like fastlane. The range could of course
include `danger` 4 up to 8, but since it's a development dependency, it
can be as high as we want it.

The reason to update was that it was holding other dependencies back,
like `kramdown` or `faraday`.

Talking about `faraday`, ironically it had to be locked to less than 1
because of two failing test cases with `faraday` 1.x. Specifically these
two:
`rspec ./spaceship/spec/portal/enterprise_spec.rb:21`
`rspec ./spaceship/spec/portal/portal_client_spec.rb:286`

I believe at least one of them is caused by `webmock` in combination
with `faraday` and empty arrays as parameters. See also the changelog of
`webmock` for version 3.6, which supposedly fixes this. However I
haven't tried since this would entail updating `webmock`.

The `faraday` dependency was initially loosened in a PR, however to no
`Gemfile.lock` being committed, it probably remained largely unnoticed
that another dependency (`danger`) was holding it back.

https://github.com/danger/danger/blob/v8.0.2/CHANGELOG.md#800
https://github.com/bblimke/webmock/blob/v3.8.3/CHANGELOG.md#360
#16399
#8135

Further reading about committing a `Gemfile.lock` file:
https://bundler.io/blog/2018/01/17/making-gem-development-a-little-better.html
@fastlane fastlane locked and limited conversation to collaborators Jul 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants