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
Fixed type error in 'brew audit' #14808
Conversation
Review period will end on 2023-02-28 at 00:00:00 UTC. |
Review period skipped due to |
@@ -528,7 +527,7 @@ def audit_signing | |||
end | |||
end | |||
|
|||
sig { returns(T.nilable(T.any(T::Boolean, Symbol))) } | |||
sig { returns(T.any(NilClass, T::Boolean, Symbol)) } |
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.
This is equivalent but marginally more readable imo (one fewer nesting of parens)
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.
This sort of weird multi-typing is probably something to address at some point (what's the difference between nil
and false
? when is true
returned over Symbol
?). Cask is littered with it so it's a big task to clean up everywhere.
There's a lot of nilable boolean stuff that is also very weird in places. It make sense for certain triple-state CLI arguments where explicit yes, explicit no and implicit default intentionally have three diferent behaviours. But some other bits I'm less sure it's necessary.
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.
Makes sense to get to typed: true
but I agree with @Bo98 that in general I think we accept/return nil
way more often than we should.
Thanks again @dduugg! |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?Resolves:
(I made the other necessary changes in oder to enable
typed: true
, which I consider to be a form of writing new tests, so I'm checking that box 😄)