-
-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Enable strict typing in Formula #19323
base: master
Are you sure you want to change the base?
Conversation
84d35c8
to
0fa08df
Compare
0fa08df
to
3d5cb6c
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.
😍
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.
Looks good to me once 🟢! Feels almost certain to go 💥 a bit after merge so let's wait until not too near to a (Monday, usually) release tag and be around for a few hours "babysitting" if possible 🙇🏻
The only (optional/non-blocking) preference I'd have here is to use T.must
less and instead do e.g. local assignment and early returns because it's a bit more clear and nicer, in my opinion.
@@ -249,7 +249,7 @@ def self.run_checks( | |||
# comparison. | |||
current = if formula | |||
if formula.head_only? | |||
Version.new(formula.any_installed_version.version.commit) | |||
Version.new(T.must(T.must(formula.any_installed_version).version.commit)) |
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.
I opened #19338 to account for these nilable values instead of using T.must
. It also adds type signatures to related methods in the process, so brew typecheck
will identify this issue as expected (shout if any of it conflicts with this PR).
Edit: Now merged, so this can be addressed in a rebase.
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?