[libcxx-commits] [PATCH] D144822: [libc++][ranges] P2711R1 Making multi-param constructors of views explicit

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D144822#4215789 <https://reviews.llvm.org/D144822#4215789>, @philnik wrote:

> In D144822#4215605 <https://reviews.llvm.org/D144822#4215605>, @H-G-Hristov wrote:
>
>> @philnik @Mordante
>>
>> Thank you for the reviews!
>>
>> I'd like to confirm what would be appropriate course of action on my side at this point in general:
>>
>> 1. I believe I addressed all remarks (IMO).
>> 2. I fixed all related CI errors but there is still one unrelated (IMO).
>> 3. I got LGTM but I still made changes after that.
>>
>> In the context of the above at this point:
>>
>> 1. Shall I request a re-review explicitly.
>> 2. Shall I wait for another review.
>> 3. Shall I decide on my own if I am good to land the patch or wait for 1) or 2)
>
> It's generally a judgement call on your side. It depends on how much the patch changed. In this case IMO it would be OK for you to just land the patch, but it's also OK if you ask again.

+1 but in general you're free to commit, unless you make "large" changes to fix the CI. That's a judgement call.

Note if I (or a different reviewer) disagrees with the original approval they can request change (the red circle with a cross next to the review), which means a new approval is required. In this case I deemed the changes small enough that I trust you to address them correctly and it doesn't need another review.

I took a look at the changes you made and I was right to trust you :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144822