damccorm commented on PR #40096: URL: https://github.com/apache/beam/pull/40096#issuecomment-5624564520
> I did this because I realize we commit multiple times during a single run of the workflow, e.g. in processNewPrs.ts, a single PR with 2 labels triggers 3 separate commits (writePrState + 2 × writeReviewersForLabelState) seconds apart. We now record a timestamp on PR creation and will make it worse. Does this actually cause problems though? 2 (or 10) small commits are not really meaningfully different than a single large commit here. > To avoid merge conflict we can add the 3 pr-bot workflow into a concurrency_group (pr-bot-prs-needing-attention.yml, pr-bot-pr-updates.yml, pr-bot-new-prs.yml). Checking running history all of them are running for 1-2 minutes. If number of PR comments is less than "~50" per hour it's managable. This seems less desirable to me. In particular, there are some issues with slow pr-bot-pr-updates. As an example, consider: 1. pr-bot-new-prs gets triggered but queued 2. I comment `R: foo` and mark my PR ready for review (not draft) Today, there is a small chance of the PR getting assigned before the `R:` comment is processed (you'd need pr-bot-new-prs to have pathological timing). If there is backlog, the odds get much worse with this change. I think this is ok if there is a problem we're solving, but I don't think we're actually seeing poor behavior. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
