-
Notifications
You must be signed in to change notification settings - Fork 55
Check if existing comment contains "Squawk Report" header. #477
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
Check if existing comment contains "Squawk Report" header. #477
Conversation
Closes sbdchd#476 When searching for an existing comment to overwrite, check the body of each comment to see if it includes the expected header. This should prevent Squawk from accidentally overwriting comments generated by other GHAs.
👷 Deploy request for squawkhq pending review.Visit the deploys page to approve it
|
| match comments.iter().find(|x| { | ||
| x.user.r#type == "Bot" | ||
| && x.user.login == bot_name | ||
| && x.body.contains(existing_comment_text_includes) |
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 thought about using starts_with here to be even more specific, but I wasn't sure how that would play with the markdown generation
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.
ah is the issue that there's one bot account setup and multiple apps, other than squawk, use it?
Edit: read your issue, makes sense!
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.
can we add a comment explaining the reasoning? otherwise looks good!
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.
Added!
sbdchd
left a comment
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.
Looks good, one minor comment about adding a comment!
| match comments.iter().find(|x| { | ||
| x.user.r#type == "Bot" | ||
| && x.user.login == bot_name | ||
| && x.body.contains(existing_comment_text_includes) |
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.
can we add a comment explaining the reasoning? otherwise looks good!
sbdchd
left a comment
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.
Thank you!!
Closes #476
When searching for an existing comment to overwrite, check the body of each comment to see if it includes the expected header. This should prevent Squawk from accidentally overwriting comments generated by other GHAs.