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

Allow for custom sorbet executables via TPC_SORBET_EXE env var #246

Merged
merged 9 commits into from
Mar 7, 2021

Conversation

nathanmsmith
Copy link
Contributor

@nathanmsmith nathanmsmith commented Mar 4, 2021

Motivation

Fixes #245. Allows users to specify an alternative installation path for sorbet-static via TPC_SORBET_EXE, similar to the SRB_SORBET_EXE used by srb.

Implementation

The path to sorbet-static was hardcoded in lib/tapioca/compilers/sorbet.rb. I added a conditional to check if TPC_SORBET_EXE is set, and if so use that value instead. If TPC_SORBET_EXE is not set, behavior should be unchanged.

I wasn't sure where/if I should add documentation to let users know TPC_SORBET_EXE exists. Open to suggestions!

Tests

Unit tests added in spec/tapioca/compilers/sorbet_spec.rb. I also ran my fork of tapioca on my system with my custom sorbet executable, to confirm it works. Not sure if it makes sense to add any integration or e2e tests for this change.

@ghost ghost added the cla-needed label Mar 4, 2021
@ghost ghost removed the cla-needed label Mar 4, 2021
Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to put this together. I have a few comments for improvement that I've noted inline.

Also, seeing TPC_ repeated, I decided that it doesn't look nice. Explicitly writing out TAPIOCA_ for the env var prefix will be much better.

lib/tapioca/compilers/sorbet.rb Outdated Show resolved Hide resolved
spec/tapioca/compilers/sorbet_spec.rb Outdated Show resolved Hide resolved
spec/tapioca/compilers/sorbet_spec.rb Outdated Show resolved Hide resolved
spec/tapioca/compilers/sorbet_spec.rb Outdated Show resolved Hide resolved
Copy link
Member

@paracycle paracycle left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

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.

Support alternative Sorbet install locations
2 participants