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

Fix Ruby 2.7 keyword args deprecation in DSL #1176

Merged
merged 2 commits into from Apr 8, 2020

Conversation

JonRowe
Copy link
Member

@JonRowe JonRowe commented Apr 5, 2020

Fixes #1150, replaces #1154

@JonRowe JonRowe force-pushed the fix-kw-args-deprecation-in-dsl branch 2 times, most recently from 0f7d1d9 to 1ed0a75 Compare April 5, 2020 11:50
@JonRowe JonRowe requested a review from pirj April 5, 2020 16:12
@pirj
Copy link
Member

pirj commented Apr 5, 2020

This change doesn't seem to fix the original issue:

    RSpec::Matchers.define(:be_foo) do |bar: nil|
      match { expect(true).to be true }
    end

    it 'works' do
      expect(:foo).to be_foo(bar: :qux)
    end

@JonRowe
Copy link
Member Author

JonRowe commented Apr 5, 2020

Oh hmph, then the original implementation can't have fixed it either.

@JonRowe JonRowe requested a review from benoittgt April 5, 2020 19:28
@JonRowe JonRowe force-pushed the fix-kw-args-deprecation-in-dsl branch from 1ed0a75 to 94c1d76 Compare April 8, 2020 09:06
@JonRowe
Copy link
Member Author

JonRowe commented Apr 8, 2020

Added additional specs and fix.

Copy link
Member

@benoittgt benoittgt left a comment

Choose a reason for hiding this comment

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

LGTM Jon, but I have one question.

spec/rspec/matchers/dsl_spec.rb Show resolved Hide resolved
@JonRowe JonRowe force-pushed the fix-kw-args-deprecation-in-dsl branch from 94c1d76 to d25c75c Compare April 8, 2020 13:46
@JonRowe
Copy link
Member Author

JonRowe commented Apr 8, 2020

Happy with this @pirj?

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

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

No warnings! 👍 👏

@JonRowe JonRowe merged commit e48f774 into master Apr 8, 2020
@JonRowe JonRowe deleted the fix-kw-args-deprecation-in-dsl branch April 8, 2020 17:17
@lostie
Copy link

lostie commented May 7, 2020

Hi there 👋 , when are you planning to release a new version of the gem that includes these changes?

@JonRowe
Copy link
Member Author

JonRowe commented May 7, 2020

As a volunteer project we don't have a timescale sorry. Soon, theres some other keyword warning removals in progress.

@pirj
Copy link
Member

pirj commented May 7, 2020

@lostie It would be very helpful if you could run RSpec's master against your project and open a ticket if you happen to experience more of those warnings.

@marcandre
Copy link
Contributor

@JonRowe a release would be appreciated, even if it doesn't resolve all issues, it resolves some of them. IMO, the time to create a release is so small compared to just about anything involving maintenance of a gem... when you have a great tests suite like rspec does, frequent bug fix releases make a lot of sense to me.

@marcandre
Copy link
Contributor

FWIW, this resolves the only warnings I had in 2.7 🎉

I had to modify my Gemfile with:

gem 'rspec', git: 'https://github.com/rspec/rspec.git'
gem 'rspec-expectations', git: 'https://github.com/rspec/rspec-expectations.git'
gem 'rspec-core', git: 'https://github.com/rspec/rspec-core.git'
gem 'rspec-mocks', git: 'https://github.com/rspec/rspec-mocks.git'
gem 'rspec-support', git: 'https://github.com/rspec/rspec-support.git'

I'm betting 0.25$ that figuring this out took me more than it takes to make a release.

@JonRowe
Copy link
Member Author

JonRowe commented May 7, 2020

@marcandre yes I plan to do one soon, the pattern is generally to try to batch releases together but not wait too long, after all too frequent releases are exhausting for both user and maintainer.

However I haven't had any "not tired" time since this was re highlighted and I don't do releases when I'm tired because I tend to make mistakes that are then very hard to reverse in a project as widely used as RSpec.

FWIW theres a bunch that goes into releasing one of the RSpec gems, checking any dependencies on the other gems, going through master and checking its been properly merged into 3-9-maintenance, then finally bumping the release, pushing it up, waiting for the build, then cutting the release is the final step.

JonRowe added a commit that referenced this pull request May 8, 2020
Fix Ruby 2.7 keyword args deprecation in DSL
JonRowe added a commit that referenced this pull request May 8, 2020
JonRowe added a commit that referenced this pull request May 8, 2020
@JonRowe
Copy link
Member Author

JonRowe commented May 8, 2020

This has been released as 3.9.2, I apologise for the delay but for your, 25¢ it took a good 45 minutes to check and sort the status of the two branches, get everything built and released, so I really hope it took you less time to figure out how to use the master branches 😂

@marcandre
Copy link
Contributor

Thanks @JonRowe I owe you 25¢ 😆
I'll make that a beer if we get the chance to cross paths!

@lostie
Copy link

lostie commented May 11, 2020

Thank you @JonRowe !

yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
…rspec/fix-kw-args-deprecation-in-dsl

Fix Ruby 2.7 keyword args deprecation in DSL

---
This commit was imported from rspec/rspec-expectations@fdbf174.
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
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.

Warnings for custom matchers with keyword args on Ruby 2.7
5 participants