[Refactor] Use "case" instead of multi "if" #1332
Conversation
| "_sunos") | ||
| NVM_CPU_CORES="$(psrinfo | wc -l)";; | ||
| "_aix") | ||
| NVM_CPU_CORES="$(lsconf | command grep 'Number Of Processors:'| command awk '{print $4}')";; |
There was a problem hiding this comment.
If the contents of the case are not on the same line as the ), then the closing ;; should be on a line by itself.
There was a problem hiding this comment.
Updated, thanks.
PeterDaveHello
force-pushed
the
refactor
branch
from
09cedfa
to
673c682
Compare
| NVM_CPU_CORES="$(psrinfo | wc -l)" | ||
| ;; | ||
| "_aix") | ||
| NVM_CPU_CORES="$(lsconf | command grep 'Number Of Processors:'| command awk '{print $4}')" |
There was a problem hiding this comment.
heads up, this is going to conflict with #1319 which is being merged shortly.
There was a problem hiding this comment.
I think we both can update this part immediately, right?
There was a problem hiding this comment.
It's merged; feel free to rebase
PeterDaveHello
force-pushed
the
refactor
branch
from
673c682
to
fb230b2
Compare
|
@PeterDaveHello what about my question above? |
|
@ljharb oops, is there any reference about the difference? I wonder if it's that significant that we should worry about it, use so many if/else may not be a good implement, and also, I want to add one more OS type in |
ljharb
changed the title
|
Why would we want to disable the binary install for alpine linux? |
|
Just like: |
|
Not all of them, but a good percentage, yes - at least, that's what I'm advocating for. |
|
Then that's the part once they provided the binary for Alpine, before that, the behavior should be like FreeBSD, but no matter which one, I think we don't need to bundle with this PR, not the same issue. |
This can also help us use the more flexible pattern matching in "case" in future.