On Wed, May 21, 2014 at 5:21 PM, Alp Toker <[email protected]> wrote: > > On 22/05/2014 03:04, Chandler Carruth wrote: > > >> On Wed, May 21, 2014 at 5:38 PM, Alp Toker <[email protected] <mailto: >> [email protected]>> wrote: >> >> On 21/05/2014 23:19, David Majnemer wrote: >> >> Author: majnemer >> Date: Wed May 21 15:19:59 2014 >> New Revision: 209319 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=209319&view=rev >> Log: >> Sema: Implement DR244 >> >> Summary: >> Naming the destructor using a typedef-name for the class-name is >> well-formed. >> >> This fixes PR19620. >> >> Reviewers: rsmith, doug.gregor >> >> >> Did Doug participate in review for this patch? >> >> >> No, Richard Smith did. I see a pretty complete review thread for the >> DR244 patch in my inbox. I've even checked and none of the emails were lost >> in the recent email list snafu. >> > > Take a look at the Reviewers line? > > > >> Looking through SVN history, I'm seeing an alarming number of >> confusing review trails. >> >> >> Can you point them out specifically? I too keep a pretty close eye on >> these kinds of things and I'm not seeing any examples. >> >> >> If review happened off-list that's fine, but it needs to be stated >> clearly because the system works on trust. >> >> >> I don't think off-list review is fine... It happens some times, for good >> or bad reasons, and its not the end of the world. But it is definitely not >> SOP, and I haven't seen any evidence of it becoming more pervasive. If you >> see it, call it out. When patches merit pre-commit review, they should get >> it from the whole community. >> > > I'd expect that off-list review would be mentioned explicitly rather than > everyone making the implicit assumption that's the case. > > In the case of r209319, Doug is mentioned as reviewer but I only see > Richard's review comments on the list. > > That's why I asked for clarification on Doug's participation -- was it > off-list, or is it a mistake in the commit log? > > > >> (In fact, I'm seeing relatively inactive developer names showing >> up suspiciously in these "Reviewers" lines while some of the most >> active reviewers barely appear at all. Could this be a problem >> with Phabricator or some internal system you guys are using?) >> >> >> I'm really not sure what you're worried about here. Again, specific >> examples? >> > > So I want to know if Doug was involved with review of this change as > stated in the commit log. That's a good place to start. >
The "Reviewers" line that (IIUC) arcanist adds lists the reviewers in Phab, which includes Doug, and doesn't indicate that anyone mentioned actually reviewed anything. This is a list of people who were asked to review, not a list of people who did so. > (No sleight towards David, we're working together on plenty of stuff day > to day. It's just good practice to keep a reasonable paper trail for > authorship / reviewership and ask questions when things look out of place.) > > Alp. > > > > -- > http://www.nuanti.com > the browser experts > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
