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

Use RSpec.current_scope #2511

Merged
merged 1 commit into from
Feb 9, 2022
Merged

Use RSpec.current_scope #2511

merged 1 commit into from
Feb 9, 2022

Conversation

pirj
Copy link
Member

@pirj pirj commented Jul 30, 2021

It was introduced in rspec/rspec-core#2895

When RSpec.current_scope is released in 3.11 and 4.0, we're fine with merging this PR to main and to 5-0-maintenance.

TODO:

  • remove the temp! commit

@pirj pirj self-assigned this Jul 30, 2021
@pirj pirj requested review from JonRowe and benoittgt July 30, 2021 17:46
@pirj pirj marked this pull request as ready for review July 30, 2021 17:46
Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

For when the other PR is merged, this gets a thumbs up (without the temp pin commit of course).

@pirj
Copy link
Member Author

pirj commented Aug 15, 2021

If I merge now, I'll surely forget to cherry-pick to 5-0-maintenance.

If I cherry-pick it to 5-0-maintenance, I would have to bump expected_rspec_version in gemspec to the yet unreleased 3.11.0.

In theory, pointing to 3.11.0 might break someone's setup if they prefer using 5-0-maintenance with the latest unreleased fixes to 5.0.x.
In practice, though, there are no changes since 5.0.2, so for those using github: ..., branch ... there's a workaround to use ~> 5.0.

If I forget to cherry-pick to 5-0-maintenance, it will only be available for the future 6.0 release. Not a big deal, though.

This process requires thinking and having good memory.
I intend to hold off merging this until 3.11.0 is released, does this sound reasonable?

@JonRowe
Copy link
Member

JonRowe commented Aug 19, 2021

I don't think this should be merged to 5-0-maintenance as I don't think its a good idea to bump a dependency in a patch version, only a minor. However this also would gate any release of rspec-rails 5.1 on 3.11 so its probably still worth holding off merging.

@pirj pirj added this to the 6.0 milestone Dec 13, 2021
@JonRowe JonRowe mentioned this pull request Jan 18, 2022
21 tasks
@JonRowe JonRowe merged commit 620a869 into main Feb 9, 2022
@JonRowe JonRowe deleted the use-rspec-current_scope branch February 9, 2022 11:50
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.

None yet

2 participants