[PATCH] D80497: Make mlir::Value's bool conversion operator explicit

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat May 30 21:46:03 PDT 2020


dblaikie added a comment.

In D80497#2065134 <https://reviews.llvm.org/D80497#2065134>, @mehdi_amini wrote:

> In D80497#2064999 <https://reviews.llvm.org/D80497#2064999>, @dblaikie wrote:
>
> > In D80497#2064811 <https://reviews.llvm.org/D80497#2064811>, @ftynse wrote:
> >
> > > > I don't agree - the absence of a "this was committed without approval" is, I think, too subtle a signal for the mailing list/all other developers to take as a clear indication that a patch was reviewed. The mailing list is meant to be the system of record here, and Phabricator is a convenient tool to facilitate using it - but review communications must be on the mailing list.
> > >
> > > Well, it depends on the base assumption you make, which seems to be "anything is considered unreviewed and unapproved unless explicitly stated otherwise".
> >
> >
> > If it's been sent for review, yes - it's unapproved until it's... approved. That's what sending something for review is all about. If something is sent for review, then committed without it - that's a problem (
>
>
> Not anymore: I have seen more and more reviews created for the sake of having the pre merge testing.


If those are going to the mailing list - I don't think that's appropriate (it's noise for an already very busy mailing list, and it muddies the water about what threads need review and which ones are to be ignored). & if they don't go to the mailing list, I think that's fine - they're not reviews as far as I'm concerned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80497





More information about the llvm-commits mailing list