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

Scrape out monkey patching #2803

Merged
merged 2 commits into from
Dec 26, 2020
Merged

Scrape out monkey patching #2803

merged 2 commits into from
Dec 26, 2020

Conversation

pirj
Copy link
Member

@pirj pirj commented Dec 19, 2020

Sibling PRs:

Prerequisite PRs:

http://rspec.info/blog/2013/07/the-plan-for-rspec-3/#zero_monkey_patching_mode

we do want to encourage people to switch to the new syntax, so we plan to make RSpec 3 print a warning on first usage of any the old syntax methods (should, should_not, should_receive, etc) unless the should syntax has been explicitly enabled. This should nudge folks towards the new syntax while keeping RSpec friendly to new users and will pave the way for the old syntax to be disabled by default in RSpec 4.

zero-monkey-patching mode for RSpec... We plan for these config options to become the defaults in RSpec 4.0, so that RSpec 4.0 will have zero monkey patching out of the box.

As for "disabled by default" vs "completely removed" and "default, out of the box" vs "impossible" I can only say that RSpec 4 was probably planned to be released earlier, as:

we'll probably be dropping support for 1.8.7 in RSpec 4

but we've also dropped 1.9, 2.0, 2.1 and 2.2

#2301 (comment)

In RSpec 4, we plan to extract all monkey patching from RSpec and move it into a separate gem, so that monkey patching is opt-in instead of opt-out and users have to explicitly install and load a gem to get it.

rspec-should (or rspec-monkey as it's also about exposing example group DSL in the top-level/Module?) will be released later.

Those using the monkey-patched should syntax are not encouraged to update to RSpec 4 until this gem is extracted.

Those using the globally-exposed DSL are encouraged to use RSpec.describe/RSpec.shared_examples_for instead.

spec/rspec/core/configuration_spec.rb Show resolved Hide resolved
spec/rspec/core/configuration_spec.rb Show resolved Hide resolved
spec/rspec/core/dsl_spec.rb Show resolved Hide resolved
features/subject/one_liner_syntax.feature Outdated Show resolved Hide resolved
@pirj pirj force-pushed the remove-monkey-patching branch 2 times, most recently from a93a956 to d2f201f Compare December 23, 2020 16:10
Changelog.md Outdated Show resolved Hide resolved
features/subject/one_liner_syntax.feature Outdated Show resolved Hide resolved
features/subject/one_liner_syntax.feature Outdated Show resolved Hide resolved
features/subject/one_liner_syntax.feature Outdated Show resolved Hide resolved
remove-monkey-patching
Copy link
Member

Choose a reason for hiding this comment

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

Is there a need for parity? whats the Pr ordering here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Core first, then Mocks or Expectations no matter the order.

Copy link
Member

Choose a reason for hiding this comment

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

Then as this is core, this should be dropped?

Copy link
Member Author

Choose a reason for hiding this comment

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

Rolled it back.

@pirj pirj requested a review from JonRowe December 23, 2020 23:41
@pirj pirj dismissed JonRowe’s stale review December 23, 2020 23:41

Code review notes addressed

@pirj
Copy link
Member Author

pirj commented Dec 24, 2020

Build failure is expected. Expectations call the removed config.disable_monkey_patching! method in their specs.

@JonRowe
Copy link
Member

JonRowe commented Dec 25, 2020

Ok, so this is evidently dependant on the others, which of the others can be merged without core? It seems like they should be able to be, but I haven't worked on it. When pinning repositories @pirj can you please explain what will fail without it? If they are all interdependant none of them can be merged until they are all ready for review.

@pirj
Copy link
Member Author

pirj commented Dec 25, 2020

they are all interdependant none of them can be merged until they are all ready for review.

They are interdependent, and they are all ready for review.

@pirj pirj added this to the 4.0 milestone Dec 25, 2020
@pirj
Copy link
Member Author

pirj commented Dec 26, 2020

@JonRowe

/home/runner/work/rspec-core/rspec-expectations/bin/rspec

An error occurred while loading spec_helper.
Failure/Error: config.disable_monkey_patching!

Do you suggest to send a PR to rspec-expectations's against 4-0-dev to:

- config.disable_monkey_patching!
+ config.disable_monkey_patching! if config.respond_to?(:disable_monkey_patching!)

?

@pirj
Copy link
Member Author

pirj commented Dec 26, 2020

Green 💚

http://rspec.info/blog/2013/07/the-plan-for-rspec-3/#zero_monkey_patching_mode

> we do want to encourage people to switch to the new syntax, so we plan to make RSpec 3 print a warning on first usage of any the old syntax methods (should, should_not, should_receive, etc) unless the should syntax has been explicitly enabled. This should nudge folks towards the new syntax while keeping RSpec friendly to new users and will pave the way for the old syntax to be disabled by default in RSpec 4.

> zero-monkey-patching mode for RSpec...  We plan for these config options to become the defaults in RSpec 4.0, so that RSpec 4.0 will have zero monkey patching out of the box.

As for "disabled by default" vs "completely removed" and "default, out
of the box" vs "impossible" I can only say that RSpec 4 was probably planned to
be released earlier, as:

> we'll probably be dropping support for 1.8.7 in RSpec 4

but we've also dropped 1.9, 2.0, 2.1 and 2.2

#2301 (comment)

> In RSpec 4, we plan to extract all monkey patching from RSpec and move it into a separate gem, so that monkey patching is opt-in instead of opt-out and users have to explicitly install and load a gem to get it.

`rspec-should` (or `rspec-monkey` as it's also about exposing example
group DSL in the top-level/Module?) will be released later.

Those using the monkey-patched `should` syntax are not encouraged to
update to RSpec 4 until this gem is extracted.

Those using the globally-exposed DSL are encouraged to use
`RSpec.describe`/`RSpec.shared_examples_for` instead.
@JonRowe JonRowe merged commit 832adc1 into 4-0-dev Dec 26, 2020
@JonRowe JonRowe deleted the remove-monkey-patching branch December 26, 2020 19:44
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 19, 2021
…key-patching

Scrape out monkey patching

---
This commit was imported from rspec/rspec-core@832adc1.
pirj added a commit to pirj/discourse that referenced this pull request Jul 23, 2022
pirj added a commit to pirj/discourse that referenced this pull request Jul 24, 2022
pirj added a commit to pirj/discourse that referenced this pull request Jul 27, 2022
pirj added a commit to pirj/discourse that referenced this pull request Jul 27, 2022
tgxworld pushed a commit to discourse/discourse that referenced this pull request Jul 28, 2022
* Remove outdated option

rspec/rspec-core@0407831

* Use the non-globally exposed RSpec syntax

rspec/rspec-core#2803

* Use the non-globally exposed RSpec syntax, cont

rspec/rspec-core#2803

* Comply to strict predicate matchers

See:
 - rspec/rspec-expectations#1195
 - rspec/rspec-expectations#1196
 - rspec/rspec-expectations#1277
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.

2 participants