From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cuda.sgi.com (cuda1.sgi.com [192.48.157.11]) by oss.sgi.com (8.14.3/8.14.3/SuSE Linux 0.8) with ESMTP id pBTLj33W028462 for ; Thu, 29 Dec 2011 15:45:03 -0600 Received: from ipmail07.adl2.internode.on.net (ipmail07.adl2.internode.on.net [150.101.137.131]) by cuda.sgi.com with ESMTP id BReniQ9AZq2kjKco for ; Thu, 29 Dec 2011 13:45:00 -0800 (PST) Date: Fri, 30 Dec 2011 08:44:41 +1100 From: Dave Chinner Subject: Re: [PATCH 2/2 v2] xfs: log all dirty inodes in xfs_fs_sync_fs Message-ID: <20111229214441.GI12731@dastard> References: <20111218154936.GA17626@infradead.org> <20111218155015.GC17626@infradead.org> <20111220200841.GA2788@infradead.org> <20111223214703.GW29840@sgi.com> <20111226121302.GE12731@dastard> <20111229154207.GK21646@sgi.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20111229154207.GK21646@sgi.com> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: xfs-bounces@oss.sgi.com Errors-To: xfs-bounces@oss.sgi.com To: Ben Myers Cc: Christoph Hellwig , Paul Anderson , Sean Thomas Caron , Mark Tinguely , xfs@oss.sgi.com On Thu, Dec 29, 2011 at 09:42:07AM -0600, Ben Myers wrote: > Hi Dave, > > On Mon, Dec 26, 2011 at 11:13:02PM +1100, Dave Chinner wrote: > > On Fri, Dec 23, 2011 at 03:47:03PM -0600, Ben Myers wrote: > > > > > > Reviewed-by: Ben Myers > > > > > > Mark also reviewed this. > > > > > > Reviewed-by: Mark Tinguely > > > > Just a process note here: if Mark reviewed the code and is happy > > with it, then he needs to send his reviewed-by tag himself. If he's > > got concerns, then he needs to discuss them on the list with the > > patch author, not just in private with you. If a person's questions > > are not posted to the mailing list or posted by proxy and they > > didn't aprticipate in discussions on the list, then there is no > > evidence that the person ever reviewed the patch. Hence the tag has > > no value because it is not verifiable. > > I tend to agree that it is important to discuss things openly on the > list. Will make an effort to do more of this. > > > More importantly, tags are a semi-formal statement that a set of > > actions has been taken by that person - see > > Documentation/SubmittingPatches for the actions different tags > > imply. Hence it is important the actions they imply are verifiable, > > and it also reinforces the fact that they only have value when they > > are issued by the email address (or a known alias) in the tag.... > > I don't see anything in SubmittingPatches that says the address on the > From line not matching a tag is a dealbreaker, and I think that we > should give credit where it is due. Mark did some work to review and > understand this code in addition to his testing. But credit is not what the Reviewed-by tag means - it's a statement of fact about actions performed by the reviewer. A partial review or partaking in part of a review does not mean a person can put a "reviewed-by" tag on a commit. An Acked-by my be appropriate in that case (though I see them a worthless by their very nature), but reviewed-by tags are not for "giving credit" to other people that may have helped you. Further, keep in mind that I've only ever seen 2 emails from Mark, so I have no idea who he is or what his capabilities are, so I do not yet know how much to trust his reviews or testing. Until I've spend some time interacting with him directly, I won't be able to form those opinions, and so such tags start at the lower end of value. They have even less value when they don't come from him and he hasn't commented where I can see it. > I have him a call and > asked him if I could add a 'Reviewed-by' to his 'Tested-by' because I > was suprised he didn't... Next time I'll ask him to send it himself. So, perhaps he didn't consider what he did met all the criteria of a reviewed-by tag? See what I mean about tags being worthless when sent by proxy? > I'd like to point out that plenty of the conversation surrounding this > pair of patches seems not to have made it to the list either. Happens all the time, especially on #xfs. The difference is, though, that such conversations are not "reviews" or result in one person sending reviewed-by tags for all the participants in the conversation.... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs