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

Update RuboCop #1149

Merged
merged 1 commit into from
Apr 19, 2020
Merged

Update RuboCop #1149

merged 1 commit into from
Apr 19, 2020

Conversation

AlexWayfer
Copy link
Contributor

@AlexWayfer AlexWayfer commented Apr 17, 2020

Require its specific version in Gemfile.

Fix it's installation in CI.

Enable new cops.

Resolve new offenses.

Drop support of Ruby 2.3
(it's not supported by RuboCop and by MRI:https://www.ruby-lang.org/en/news/2019/03/31/support-of-ruby-2-3-has-ended/)

Deprecate Faraday::Request#method, replace it with #http_method.

Todos

List any remaining work that needs to be done, i.e:

  • Tests
  • Documentation

Additional Notes

It requires for neighbor pull requests.

Copy link
Member

@iMacTia iMacTia 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 taking a stab at this, we totally needed an update.
I've left some comments/change requests

lib/faraday/request.rb Show resolved Hide resolved
Gemfile Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
@AlexWayfer AlexWayfer force-pushed the update_rubocop branch 4 times, most recently from 411e16b to b327886 Compare April 19, 2020 02:28
Copy link
Member

@olleolleolle olleolleolle 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 your awesome work here. It's doing a lot of good!

I had one question, you can find it inline.

Thanks again!

script/proxy-server Outdated Show resolved Hide resolved
Copy link
Member

@iMacTia iMacTia left a comment

Choose a reason for hiding this comment

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

Thanks @AlexWayfer for addressing all my comments 😄!
And sorry if I came across as a bit fussy, we really appreciate all the improvements you've added here!

One last request (but not mandatory to merge, I can do it later if you don't have time), since we decided to rename #method into #http_method and deprecated the old method, it would be good to update the UPGRADING.md file as well to keep track of all backwards-incompatible changes.
I think this is going to be the first v2.0 note!

Require its specific version in `Gemfile`.

Fix it's installation in CI.

Enable new cops.

Resolve new offenses.

Drop support of Ruby 2.3
(it's not supported by RuboCop and by MRI:https://www.ruby-lang.org/en/news/2019/03/31/support-of-ruby-2-3-has-ended/)

Deprecate `Faraday::Request#method`, replace it with `#http_method`.
@AlexWayfer
Copy link
Contributor Author

And sorry if I came across as a bit fussy

No problem.

we really appreciate all the improvements you've added here!

Thank you, I see. 😉 And thank you for help and discussing.

One last request (but not mandatory to merge, I can do it later if you don't have time), since we decided to rename #method into #http_method and deprecated the old method, it would be good to update the UPGRADING.md file as well to keep track of all backwards-incompatible changes.
I think this is going to be the first v2.0 note!

I agree, added. If you want another content of this note — please, write, it's not too hard for me to change.

@AlexWayfer AlexWayfer requested a review from olleolleolle April 19, 2020 12:25
Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Looks good!

Thanks for sticking with it. <3

@olleolleolle olleolleolle merged commit c26df87 into lostisland:master Apr 19, 2020
@AlexWayfer AlexWayfer deleted the update_rubocop branch April 19, 2020 16:35
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