Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Empty last hash matches allow any kwargs #375

Merged
merged 1 commit into from
Apr 30, 2019

Conversation

twalpole
Copy link
Contributor

This fixes issue #374

@JonRowe
Copy link
Member

JonRowe commented Apr 21, 2019

Ok, you'll need a spec for this, also consider not everything responds to empty?...

@twalpole
Copy link
Contributor Author

twalpole commented Apr 21, 2019

@JonRowe At that point args.last is guaranteed to be a Hash so it is guaranteed to respond to empty? and there is a spec already in the PR.

@twalpole
Copy link
Contributor Author

@JonRowe - Just a friendly ping

@benoittgt
Copy link
Member

Sorry for the late review. This patch looks good to me. Because we do Hash === args.last as the first condition we are sure arg.last.empty? is done on the proper type. Also @JonRowe I don't understand which test we should add.

For the failing on appveyor. It is unrelated. https://ci.appveyor.com/project/rspec/rspec-support/builds/23996036/job/egb1o129jo145r0t

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 thanks @twalpole

@JonRowe
Copy link
Member

JonRowe commented Apr 30, 2019

Thanks :)

JonRowe added a commit that referenced this pull request Apr 30, 2019
JonRowe added a commit that referenced this pull request Jun 10, 2019
Empty last hash matches allow any kwargs
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
…low_all_kwargs

Empty last hash matches allow any kwargs

---
This commit was imported from rspec/rspec-support@7c2aef3.
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
…low_all_kwargs

Empty last hash matches allow any kwargs

---
This commit was imported from rspec/rspec-support@7c2aef3.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants