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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
