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

Drop pry as a runtime dependency #1259

Merged
merged 2 commits into from
Nov 3, 2022
Merged

Drop pry as a runtime dependency #1259

merged 2 commits into from
Nov 3, 2022

Conversation

paracycle
Copy link
Member

Motivation

We've been using pry as a runtime dependency only because it was convenient to use its Pry::WrappedModule functionality to figure out which files a constant might be defined in.

However, that's actually a very minimal amount of logic that we've been relying on and the runtime dependency on a development tool has always been a bit weird. It is about time we stop doing that.

Implementation

I mostly adapted the code from https://github.com/pry/pry/blob/master/lib/pry/wrapped_module.rb#L296-L368 into our Reflection class.

Tests

No added tests, existing tests should continue to run properly.

@paracycle paracycle requested a review from a team as a code owner November 2, 2022 19:33
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Nice 👍

lib/tapioca/runtime/reflection.rb Outdated Show resolved Hide resolved
lib/tapioca/runtime/reflection.rb Outdated Show resolved Hide resolved
lib/tapioca/runtime/reflection.rb Show resolved Hide resolved
Add relevant machinery inside the `Reflection` class to discover
file candidates for a given constant based on the source location of
the given constant's (or its singleton class') methods.
@paracycle paracycle merged commit 375610b into main Nov 3, 2022
@paracycle paracycle deleted the uk-drop-pry branch November 3, 2022 19:24
@shopify-shipit shopify-shipit bot temporarily deployed to production November 10, 2022 17:34 Inactive
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.

3 participants