-
Notifications
You must be signed in to change notification settings - Fork 499
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
Disable vacation messaging #4079
Conversation
Thanks for submitting this pull request! Some main reviewers |
@seanlip @rt4914 @anandwana001 PTAL. I'll merge this once I get one approval; just sending it broadly to increase chances of faster turnaround so that this message gets disabled. |
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.
Hi Ben -- left one suggestion, thanks!
@@ -9,6 +9,7 @@ on: | |||
jobs: | |||
holiday_message: | |||
runs-on: ubuntu-latest | |||
if: 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.
Perhaps worth adding a comment here saying something like "If situation XXX occurs, do YYY to turn this on." (It doesn't seem obvious to me as a possible future contributor that the natural thing to do is to drop this line altogether.)
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.
Good suggestion--done.
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.
LGTM, agreed with Sean's comment about adding a comment to make true/false thing more understandable.
Add comment per reviewer suggestion.
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.
Thanks, LGTM
Thanks @seanlip & @anandwana001. Enabling auto-merge. |
Explanation
Now that the team is back for reviews, the automated PR message is no longer needed. This PR keeps the messaging support, but disables the vacation-specific message used over the last few weeks.
Essential Checklist
The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)For UI-specific PRs only
N/A -- CI workflow change