-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
dev-cmd: add brew typecheck
command
#8289
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
Conversation
a4d25bf
to
64107ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, good work - and speedy work, too! 🚀
I tested this out and there's some confusing behaviour with --file
and --dir
. Thankfully though, --quiet
works and is very pleasing to my eyes. 🤣
Also: might be nice for this to have a |
bed9844
to
1f2fdc6
Compare
1f2c1ab
to
470c863
Compare
470c863
to
85b5753
Compare
This PR adds a new `brew typecheck` developer command which checks for typechecking errors in the current code with Sorbet.
85b5753
to
98f8235
Compare
.github/workflows/tapioca.yml
Outdated
cd "$GITHUB_WORKSPACE/Library/Homebrew" | ||
gem install bundler -v "~>1" | ||
bundle install --jobs 4 --retry 3 | ||
run: brew typecheck --update-definitions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Judging by the current source code, I would say this whole step is not needed.
This will allow us to verify that we can merge new sorbet/tapioca updates.
description: "Silence all non-critical errors." | ||
switch "--update-definitions", | ||
description: "Update Tapioca gem definitions of recently bumped gems" | ||
switch "--fail-if-not-changed", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this --fail-if-changed
? The double negative is kind of confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this matches -update-license-data
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or at least: yes, it could but both would need to be updated (which is out of scope for this PR specifically).
Thanks @vidusheeamoli, great work here! |
@@ -120,6 +120,9 @@ jobs: | |||
- name: Run brew audit --skip-style on all taps | |||
run: brew audit --skip-style | |||
|
|||
# TODO: remove --quiet when possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vidusheeamoli I added this so we can ensure that PRs like #8505 are tested properly. Could you open a new PR to remove --quiet
from here and fix and/or hide the current failures so that brew typecheck
shows as clear by default? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On it! 😄
brew style
with your changes locally?brew tests
with your changes locally?This PR adds a new
brew typecheck
developer command which checks for type errors in the current code with Sorbet.