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

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.

@pirj pirj self-assigned this Dec 19, 2020
@pirj pirj force-pushed the remove-monkey-patching branch 2 times, most recently from fd5fd1c to c9a5147 Compare December 19, 2020 00:27
@pirj pirj marked this pull request as ready for review December 19, 2020 00:52
@pirj pirj requested review from benoittgt and JonRowe December 19, 2020 00:53
@pirj pirj force-pushed the remove-monkey-patching branch from c9a5147 to e9f36c5 Compare December 21, 2020 23:15
@pirj pirj force-pushed the remove-monkey-patching branch from 6d0f62a to e9f36c5 Compare December 22, 2020 09:15
@pirj pirj force-pushed the remove-monkey-patching branch 2 times, most recently from a93a956 to d2f201f Compare December 23, 2020 16:10
@pirj pirj force-pushed the remove-monkey-patching branch from d2f201f to 15a7a56 Compare December 23, 2020 16:16
@pirj pirj requested a review from JonRowe December 23, 2020 17:36
@pirj pirj mentioned this pull request Nov 27, 2024
53 tasks
@pirj pirj force-pushed the remove-monkey-patching branch from 2cdc875 to 15a7a56 Compare December 23, 2020 20:49
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 force-pushed the remove-monkey-patching branch from 7e4d095 to 3069a97 Compare December 23, 2020 23:41
@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 pirj force-pushed the remove-monkey-patching branch from 3069a97 to 8dbbb8f Compare December 24, 2020 22:24
@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 pirj force-pushed the remove-monkey-patching branch from 8dbbb8f to 4af0c13 Compare December 25, 2020 14:03
@pirj pirj force-pushed the remove-monkey-patching branch from 4af0c13 to 712689e Compare December 26, 2020 16:50
@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 pirj force-pushed the remove-monkey-patching branch from 712689e to e3c5c9b Compare December 26, 2020 17:09
@pirj
Copy link
Member Author

pirj commented Dec 26, 2020

Green 💚

pirj added 2 commits December 26, 2020 22:01
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.
@pirj pirj force-pushed the remove-monkey-patching branch from e3c5c9b to c8c5356 Compare December 26, 2020 19:01
@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 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.

2 participants