Skip to content

Remove instance_eval from InspectionTree#210

Merged
mcmire merged 1 commit into
mainfrom
remove-instance-eval-from-inspection-tree
Feb 1, 2024
Merged

Remove instance_eval from InspectionTree#210
mcmire merged 1 commit into
mainfrom
remove-instance-eval-from-inspection-tree

Conversation

@mcmire
Copy link
Copy Markdown
Collaborator

@mcmire mcmire commented Feb 1, 2024

The InspectionTree class defines methods that create nodes in the tree. Some of these methods take a block which is instance_eval'ed inside of a new subtree. This use of instance_eval may seem to create a very clean and easy to use API on the surface, but it quickly breaks down if, as you are defining your InspectionTree, you need to extract methods inside of your InspectionTreeBuilder and then reference them. Not instance_eval'ing creates footguns, but it also removes the magic and mystery around how InspectionTree works.

@mcmire mcmire force-pushed the remove-instance-eval-from-inspection-tree branch 2 times, most recently from dbe1a0f to 6a3d2df Compare February 1, 2024 05:25
The InspectionTree class defines methods that create nodes in the tree.
Some of these methods take a block which is `instance_eval`'ed inside of
a new subtree. This use of `instance_eval` may seem to create a very
clean and easy to use API on the surface, but it quickly breaks down if,
as you are defining your InspectionTree, you need to extract methods
inside of your InspectionTreeBuilder and then reference them.
_Not_ `instance_eval`'ing creates footguns, but it also removes the
magic and mystery around how InspectionTree works.
@mcmire mcmire force-pushed the remove-instance-eval-from-inspection-tree branch from 6a3d2df to e672450 Compare February 1, 2024 05:50
@mcmire mcmire merged commit 181b102 into main Feb 1, 2024
@mcmire mcmire deleted the remove-instance-eval-from-inspection-tree branch February 2, 2024 04:28
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.

1 participant