Extract BaseHookCollection #1132
Conversation
| def instance_exec(*args, &block) | ||
| @example_group_instance.instance_exec(*args, &block) | ||
| def instance_exec(example_or_procsy, &block) | ||
| @example_group_instance.instance_exec(example_or_procsy, &block) | ||
| end |
There was a problem hiding this comment.
I didn't realize that we override instance_exec here. Given that it's a core ruby method available on all objects that is expected to behave in a certain way (i.e. eval the block with the object as self) and this does not behave that way...can we rename this to eval_block_in_group or something like that? I'd prefer not to stomp on ruby's version of this method.
There was a problem hiding this comment.
Also, why did you change this from a splat? It's unclear to me what the connection of this change is to the PR as a whole.
|
The changes look fine. My concerns above aren't really affected by this PR (as |
|
The explicit arg instead of splat was to clarify usage. How 'bout I back that change out of this PR and address that and wrapping instance_exec separately? |
👍 |
Previously the HookCollection was acting as a base with the subclasses overriding (some) methods. With this change, each subclass extends the base class by adding new methods, not overriding existing ones. Makes it all a bit easier to reason about.
Previously the HookCollection was acting as a base with the subclasses
overriding (some) methods. With this change, each subclass extends the
base class by adding new methods, not overriding existing ones. Makes it
all a bit easier to reason about.