[mergebot] combine land check and job started comments #80965
Conversation
🔗 Helpful links
✅ No Failures (0 Pending)As of commit 2c4084e (more details on the Dr. CI page): Expand to see more💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
There was a problem hiding this comment.
I prefer the alternative with the message being more like: "@pytorchbot successfully started a merge and is creating land time checks (which may take a minute). See merge status here and track land check progress [here]."
Reason being: if for whatever reason create_land_time_check_branch doesn't run or fails or something happens between the initial read_merge_rules to this code, the user may never get this notification (and it will be inconsistent with our other bot merge commands).
But any error would be caught by the outer exception handler and put as a comment by the bot, right? |
|
Right, though having this comment be split into two places in the code still is strange. Not a big deal either way though. |
zengk95
force-pushed
the
combine-merge-comment
branch
from
8a428e2 to
2c4084e
Compare
|
@pytorchbot merge |
|
@pytorchbot successfully started a merge job. Check the current status here |
|
Hey @zengk95. |
Summary: Addresses #80923 The alternative to this is editing the start comment to include the land checks details, but I think it might be a bit misleading because the land check branch creation might not be there when the person first clicks on it when they get a notification. This might be a bit annoying cause then the user won't be able to look at their progress. Pull Request resolved: #80965 Approved by: https://github.com/malfet Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/7ce92d7fac4da60bec6f0ab74eb911f2a8587210 Reviewed By: DanilBaibak Differential Revision: D37847409 Pulled By: zengk95 fbshipit-source-id: 88f2da5904caa48fe54ed1f3097233b914f1c9ef
6 participants
Addresses #80923
The alternative to this is editing the start comment to include the land checks details, but I think it might be a bit misleading because the land check branch creation might not be there when the person first clicks on it when they get a notification. This might be a bit annoying cause then the user won't be able to look at their progress.