-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Don't require clippy/miri for beta #50573
Conversation
# ignore $2 (branch id) | ||
verify_status $3 $4 | ||
else | ||
if [ "$2" = beta ]; then |
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 could be elif
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 think "beta" also here may want to be "beta_required"?
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.
so close, I tried elsif
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.
the "beta" here is the second arg to all function calls in status_check. It denotes the branch, not the kind of check. I should probably add more comments
verify_status $3 $4 | ||
else | ||
if [ "$2" = beta ]; then | ||
if grep -vq '"'"$1"'":"(test|build)-fail"' "$TOOLSTATE_FILE"; then |
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.
-
You should just use
-q
here. The-v
flag means inverted search -
grep
by default uses BRE which means you need to escape the brackets and vertical bars:if grep -q '"'"$1"'":"\(test\|build\)-fail"' "$TOOLSTATE_FILE"; then
or use
-E
to use ERE.
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.
done
Awesome, thanks @oli-obk! |
Locally I'm getting
if not on the nightly channel. |
@bors r+ Thanks! |
@bors r=kennytm added some docs |
🤔 bors is not seeing this PR. Closing and reopening. |
@bors r+ |
📌 Commit b817403 has been approved by |
Don't require clippy/miri for beta r? @kennytm cc @alexcrichton I'm trying this out locally atm to see if it works as I think it should. Not sure how to test it for real except wait for the next beta. fixes rust-lang#50557
Don't require clippy/miri for beta r? @kennytm cc @alexcrichton I'm trying this out locally atm to see if it works as I think it should. Not sure how to test it for real except wait for the next beta. fixes rust-lang#50557
Don't require clippy/miri for beta r? @kennytm cc @alexcrichton I'm trying this out locally atm to see if it works as I think it should. Not sure how to test it for real except wait for the next beta. fixes #50557
☀️ Test successful - status-appveyor, status-travis |
r? @kennytm
cc @alexcrichton
I'm trying this out locally atm to see if it works as I think it should. Not sure how to test it for real except wait for the next beta.
fixes #50557