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

Fix dependency sorbet runtime and sorbet #70

Merged
merged 4 commits into from
Feb 12, 2023
Merged

Conversation

ignacio-chiazzo
Copy link
Owner

@ignacio-chiazzo ignacio-chiazzo commented Feb 11, 2023

  • Make Sorbet development dependencies.
  • Make sorbet-runtime a runtime dependency.
    cc @johnnyshields

#68

@johnnyshields
Copy link

johnnyshields commented Feb 11, 2023

@ignacio-chiazzo I don't think this solves the issue, because Sorbet library is still required to run the actual code within the /lib folder. (If a library is required to run code in /lib, it should be a runtime dependency.)

Our team would like to use your SDK, but we don't want to include Sorbet into our projects. Migrating Sorbet code which is inside the .rb files to separate .rbs files would be an ideal approach for us.

@johnnyshields
Copy link

Hmmm... if its just sorbet-runtime we could try it actually...

Isn't zeitwerk still required for code loading?

@ignacio-chiazzo
Copy link
Owner Author

Hmmm... if its just sorbet-runtime we could try it actually...

Let me know how it goes. I'm down for trying migrating sigs to rbs if it makes development easier. I have been using Sorbet for 2 years and I'm very happy with it, but I understand it could not be widely used yet.

Isn't zeitwerk still required for code loading?

Right. I just updated it.

Our team would like to use your SDK, but we don't want to include Sorbet into our projects.

Let me know how can I support you.

@ignacio-chiazzo ignacio-chiazzo merged commit 422c3cc into main Feb 12, 2023
@ignacio-chiazzo ignacio-chiazzo deleted the dependency branch February 12, 2023 17:08
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.

2 participants