[PATCH] D121078: Replace links to archived mailing lists by links to Discourse forums

Thu Mar 10 10:04:34 PST 2022

aaron.ballman added a comment.

In D121078#3373081 <https://reviews.llvm.org/D121078#3373081>, @SimplyDanny wrote:

> I'm happy that you found a reasonable compromise. I like it too. ;)
>
> Now, I ask you to help me a little bit with the workflow and the test failures. The review comments are all taken care of as far as I see. One reviewer approved the changes, others are still in a "needs changes"  or an undecided state. Are approvals of all reviewers required?

You don't need all reviewers to approve a patch before landing it, but you do need to determine if there's consensus amongst the active reviewers that it's okay to land.

I think all the review comments have been addressed here, as far as I can tell, but it's usually good to double-check just to be sure when someone has explicitly marked the review as requesting changes. I usually do that by pinging the people who marked as requesting changes (or anyone else who I'm uncertain about) and giving them a day or two to respond. If there are no responses after a few days, land the changes. If it turns out there are still comments they'd like to see addressed after the patch landed, they can generally be handled post commit (or if they're a major concern, we can always revert the changes while debating what to do). A patch is not blocked by people who are in an undecided state and haven't been active in the review at all.

@ldionne @tonic -- do things look reasonable to you now?

(btw, we have some documentation on that, in case you're not seen it: https://llvm.org/docs/CodeReview.html#code-review-workflow)

> I guess, the test failures have nothing to do with my changes, or have they? Can we just ignore them if they are unrelated?

I looked at the test failures and they look unrelated, so it's fine to ignore them.

Btw, do you have commit rights for the project? If not, what name and email address would you like used for patch attribution?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121078/new/

https://reviews.llvm.org/D121078