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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
bump-formula-pr: use nil?
/blank?
/present?
in if statements
#10221
Conversation
Review period will end on 2021-01-05 at 23:33:09 UTC. |
formula ||= determine_formula_from_url(new_url) if new_url | ||
raise FormulaUnspecifiedError unless formula | ||
formula ||= determine_formula_from_url(new_url) if new_url.present? | ||
raise FormulaUnspecifiedError if formula.nil? |
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.
raise FormulaUnspecifiedError if formula.nil? | |
raise FormulaUnspecifiedError if formula.present? |
to be more properly equivalent
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.
unless formula
!= if formula.present?
Did you mean blank?
?
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.
Yes, sorry.
@@ -153,61 +153,64 @@ def bump_formula_pr | |||
|
|||
requested_spec = :stable | |||
formula_spec = formula.stable | |||
odie "#{formula}: no #{requested_spec} specification found!" unless formula_spec | |||
odie "#{formula}: no #{requested_spec} specification found!" if formula_spec.nil? |
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.
Another one I'd suggest just changing to present?
In general I'd say always use present?
or blank?
unless you're wanting to handle nil
differently to false
.
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 also a former unless
statement
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.
Yes, sorry.
08781a1
to
64d312c
Compare
Review period ended. |
Thanks @SeekingMeaning! |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?brew man
locally and committed any changes?My brain was having a hard time parsing some of these 馃槄