This repository was archived by the owner on Nov 30, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 753
Hook refactoring plus suite hook fixes #1805
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
- Move `descendant_filtered_examples.empty?` check out into the `run` method. - This allows us to only create the instances for the context hooks if we need to actually run the hooks. - Move `before_context_ivars.clear` into `run_after_context_hooks`. This gives it parity with `run_before_context_hooks`.
The runner has no business understanding the details of running the config object’s :suite hooks.
This includes new spec coverage for `(prepend|append)_(before|after)(:suite)`. Also, only reference these hooks on the config object. I plan to remove them from the `Hooks` module since they are not general purpose hooks.
...rather than delegating it to the Hooks module. This will allow us to remove :suite support from the hooks module, since it’s not something that is generally supported in other hook contexts.
The Configuration class now handles these and it’s the only place they are supported. This allows us to reduce object allocations a bit, and also clarifies the semantics of :suite hooks.
`append_after` is the registration method, not the type — the type is `after`.
5352b0b
to
900ec2c
Compare
Anyone want to review this? I just rebased and force pushed to deal with the merge conflict... /cc @rspec/rspec |
LGTM |
Thanks for the quick review, @xaviershay. I'll merge when green. |
myronmarston
added a commit
that referenced
this pull request
Dec 15, 2014
…fixes Hook refactoring plus suite hook fixes
MatheusRich
pushed a commit
to MatheusRich/rspec-core
that referenced
this pull request
Oct 30, 2020
…hook-fixes Hook refactoring plus suite hook fixes
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is some refactoring and improvements I had done during my initial implementation of #1749 but since that's not going to get merged soon I thought it worth putting this into its own PR.