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

Add deprecation warnings for features to be removed/changed in RSpec 4 #1301

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

benoittgt
Copy link
Member

@benoittgt benoittgt commented Jun 1, 2021

Adds deprecation warnings for the work done on unreleased RSpec 4.

fixes #1302

This is purposed to be released as version 3.99.

Sibling PRs:

Deprecation warnings list: #1302
Release strategy: rspec/rspec-core#2880 (comment)
RSpec 4 plan: https://github.com/rspec/rspec/issues/61
Changes: https://github.com/rspec/rspec-expectations/blob/4-0-dev/Changelog.md#development

@benoittgt benoittgt added this to the 3.99 milestone Jun 1, 2021
@benoittgt benoittgt force-pushed the rspec-should-syntqx-deprecation branch from 377fab5 to ed390ed Compare June 1, 2021 06:21
@benoittgt benoittgt force-pushed the rspec-should-syntqx-deprecation branch from c7bac85 to 46a72a3 Compare June 1, 2021 06:56
@benoittgt benoittgt requested a review from pirj June 1, 2021 08:48
@benoittgt benoittgt force-pushed the rspec-should-syntqx-deprecation branch from c03a0cd to 5eb1d10 Compare June 3, 2021 09:05
@benoittgt
Copy link
Member Author

If we clone add-major-update-deprecation-warnings from rspec/rspec-core#2880

We do not have the CI failure. I did a wip commit for the CI

"`:should` Expectations syntax",
{:call_site=>nil, :replacement=>"the default `expect` syntax"},
]
expect(RSpec).to receive(:deprecate).with(*syntax_arguments).at_least(:once)
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to add .at_least(:once) because of line 157 that leak into the test. If you comment it out, there is not need for .at_least(:once) anymore.

@benoittgt benoittgt force-pushed the rspec-should-syntqx-deprecation branch from 9478153 to 31ca861 Compare June 3, 2021 10:37
@benoittgt benoittgt modified the milestones: 3.99, 4.0 Jun 4, 2021
@benoittgt benoittgt marked this pull request as ready for review June 4, 2021 13:50
@pirj pirj mentioned this pull request Jul 16, 2021
53 tasks
@pirj pirj changed the title Add deprecation warnings for the removal of should syntax Add deprecation warnings for features to be removed/changed in RSpec 4 Jul 18, 2021
@pirj pirj modified the milestones: 4.0, 3.99 Jul 18, 2021
@pirj pirj force-pushed the rspec-should-syntqx-deprecation branch 4 times, most recently from 17e2bd3 to 8b709f1 Compare July 18, 2021 13:46
@pirj pirj added the rspec 4 label Jul 18, 2021
@pirj pirj force-pushed the rspec-should-syntqx-deprecation branch 3 times, most recently from bf684ea to b9c9c2e Compare July 18, 2021 14:04
@pirj pirj requested a review from JonRowe July 18, 2021 14:18
@pirj
Copy link
Member

pirj commented Jul 18, 2021

All three sibling PRs are green together.

@JonRowe, @benoittgt please have a look.

I suggest the following merge order:

Copy link
Member Author

@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 <3

predicate_actual = predicate_result
if value == !!predicate_actual && value != predicate_actual
RSpec.deprecate(
"`#{predicate_method_name}` returned neither `true` nor `false`, but rather `#{predicate_actual.inspect}`")
Copy link
Member Author

Choose a reason for hiding this comment

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

Good deprecation warning!

@pirj pirj force-pushed the rspec-should-syntqx-deprecation branch 2 times, most recently from 4fd9b36 to d67e751 Compare July 24, 2021 06:06
Copy link
Member Author

@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.

Still good to me

@pirj pirj removed their request for review July 27, 2021 07:04
@pirj pirj force-pushed the rspec-should-syntqx-deprecation branch from d67e751 to 800f5e4 Compare November 20, 2022 19:52
@pirj pirj force-pushed the rspec-should-syntqx-deprecation branch from 800f5e4 to c75a966 Compare November 20, 2022 19:58
@benoittgt
Copy link
Member Author

benoittgt commented Nov 22, 2022

Does it need work to make the CI green?

--
Edit:
I mean. Can I help?

@pirj
Copy link
Member

pirj commented Nov 22, 2022

It should become green when a corresponding rspec-core PR is merged. All three are green locally together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add deprecations warnings for RSpec 4
2 participants