From nobody Mon Feb 9 03:52:33 2026 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org ARC-Seal: i=1; a=rsa-sha256; t=1576185495; cv=none; d=zohomail.com; s=zohoarc; b=VCZ5qWXuF7VIC58gH98s5ULPPA9SN8YN2Sli351YIeP208gG6pt/GkThXqxDIRFyVRo+9qpsJowjGF9WajLONzYbXnm7W3mlje58aWdaG3mc2NSQuFTt5wHdLpWtmG1NRd0M2wKXnKe55c4NK5nl+W2rMlXpdZlPR62/aFgaIHs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1576185495; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=C636YF0se3dF0hjZ12DrZVjTXvlq8ynlc1dlZ0piwTs=; b=X96SMlNQacaMQjezUSni26VeKCuZcZ8TUxy+qNDa10k+sj4cMSj7zgnavQq57eEY2DMw2hqUrsplXvANawv5IZGgS/litk0pdw9Gmy+dCuKVtE6UZn//AMlHpP853UIvOG0b45sB5CErMu86ZtfU7gg8Q1Fuam5p4SaEA7lYFIQ= ARC-Authentication-Results: i=1; mx.zohomail.com; spf=none (zohomail.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 157618549586986.81066520215393; Thu, 12 Dec 2019 13:18:15 -0800 (PST) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ifVq2-0007N4-Fs; Thu, 12 Dec 2019 21:17:34 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ifVq1-0007Mc-Ev for xen-devel@lists.xenproject.org; Thu, 12 Dec 2019 21:17:33 +0000 Received: from mail.xenproject.org (unknown [104.130.215.37]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 76424bd8-1d24-11ea-a914-bc764e2007e4; Thu, 12 Dec 2019 21:15:02 +0000 (UTC) Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ifVnX-000165-Re; Thu, 12 Dec 2019 21:14:59 +0000 Received: from localhost ([127.0.0.1] helo=localhost.localdomain) by xenbits.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1ifVnX-0000Bm-MC; Thu, 12 Dec 2019 21:14:59 +0000 X-Inumbo-ID: 76424bd8-1d24-11ea-a914-bc764e2007e4 From: Lars Kurth To: xen-devel@lists.xenproject.org Date: Thu, 12 Dec 2019 21:14:34 +0000 Message-Id: X-Mailer: git-send-email 2.13.0 In-Reply-To: References: In-Reply-To: References: Subject: [Xen-devel] [PATCH v3 7/7] Added Resolving Disagreement X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Lars Kurth , xen-api@lists.xenproject.org, minios-devel@lists.xenproject.org, committers@xenproject.org, mirageos-devel@lists.xenproject.org, win-pv-devel@lists.xenproject.org MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" From: Lars Kurth This guide provides Best Practice on identifying and resolving common classes of disagreement Changes since v2 (added in v2) * Fix typos * Add section: "Issue: Multiple ways to solve a problem" * Changed line wrapping to 80 characters * Replaced inline style links with reference style links Signed-off-by: Lars Kurth -- Cc: minios-devel@lists.xenproject.org Cc: xen-api@lists.xenproject.org Cc: win-pv-devel@lists.xenproject.org Cc: mirageos-devel@lists.xenproject.org Cc: committers@xenproject.org --- resolving-disagreement.md | 188 ++++++++++++++++++++++++++++++++++++++++++= ++++ 1 file changed, 188 insertions(+) create mode 100644 resolving-disagreement.md diff --git a/resolving-disagreement.md b/resolving-disagreement.md new file mode 100644 index 0000000..97bca7b --- /dev/null +++ b/resolving-disagreement.md @@ -0,0 +1,188 @@ +# Resolving Disagreement + +This guide provides Best Practice on resolving disagreement, such as +* Gracefully accept constructive criticism +* Focus on what is best for the community +* Resolve differences in opinion effectively + +## Theory: Paul Graham's hierarchy of disagreement + +Paul Graham proposed a **disagreement hierarchy** in a 2008 essay +**[How to Disagree][1]**, putting types of arguments into a seven-point +hierarchy and observing that *moving up the disagreement hierarchy makes p= eople +less mean, and will make most of them happier*. Graham also suggested that= the +hierarchy can be thought of as a pyramid, as the highest forms of disagree= ment +are rarer. + +| ![Graham's Hierarchy of Disagreement][2] = | +| *A representation of Graham's hierarchy of disagreement from [Loudacris]= [3] + modified by [Rocket000][4]* = | + +In the context of the Xen Project we strive to **only use the top half** o= f the +hierarchy. **Name-calling** and **Ad hominem** arguments are not acceptable +within the Xen Project. + +## Issue: Scope creep + +One thing which occasionally happens during code review is that a code rev= iewer +asks or appears to ask the author of a patch to implement additional +functionalities. + +This could take for example the form of +> Do you think it would be useful for the code to do XXX? +> I can imagine a user wanting to do YYY (and XXX would enable this) + +That potentially adds additional work for the code author, which they may = not +have the time to perform. It is good practice for authors to consider such= a +request in terms of +* Usefulness to the user +* Code churn, complexity or impact on other system properties +* Extra time to implement and report back to the reviewer + +If you believe that the impact/cost is too high, report back to the review= er. +To resolve this, it is advisable to +* Report your findings +* And then check whether this was merely an interesting suggestion, or som= ething + the reviewer feels more strongly about + +In the latter case, there are typically several common outcomes +* The **author and reviewer agree** that the suggestion should be implemen= ted +* The **author and reviewer agree** that it may make sense to defer + implementation +* The **author and reviewer agree** that it makes no sense to implement the + suggestion + +The author of a patch would typically suggest their preferred outcome, for +example +> I am not sure it is worth to implement XXX +> Do you think this could be done as a separate patch in future? + +In cases, where no agreement can be found, the best approach would be to g= et an +independent opinion from another maintainer or the project's leadership te= am. + +## Issue: [Bikeshedding][5] + +Occasionally discussions about unimportant but easy-to-grasp issues can le= ad to +prolonged and unproductive discussions. The best way to approach this is to +try and **anticipate** bikeshedding and highlight it as such upfront. Howe= ver, +the format of a code review does not always lend itself well to this appro= ach, +except for highlighting it in the cover letter of a patch series. + +However, typically Bikeshedding issues are fairly easy to recognize in a c= ode +review, as you will very quickly get different reviewers providing differi= ng +opinions. In this case it is best for the author or a reviewer to call out= the +potential bikeshedding issue using something like + +> Looks we have a bikeshedding issue here +> I think we should call a quick vote to settle the issue + +Our governance provides the mechanisms of [informal votes][6] or +[lazy voting][7] which lend themselves well to resolve such issues. + +## Issue: Small functional issues + +The most common area of disagreements which happen in code reviews, are +differing opinions on whether small functional issues in a patch series ha= ve to +be resolved or not before the code is ready to be submitted. Such disagree= ments +are typically caused by different expectations related to the level of +perfection a patch series needs to fulfil before it can be considered read= y to +be committed. + +To explain this better, I am going to use the analogy of some building wor= k that +has been performed at your house. Let's say that you have a new bathroom +installed. Before paying your builder the last instalment, you perform an +inspection and you find issues such as +* The seals around the bathtub are not perfectly even +* When you open the tap, the plumbing initially makes some loud noise +* The shower mixer has been installed the wrong way around + +In all these cases, the bathroom is perfectly functional, but not perfect.= At +this point you have the choice to try and get all the issues addressed, wh= ich in +the example of the shower mixer may require significant re-work and potent= ially +push-back from your builder. You may have to refer to the initial statemen= t of +work, but it turns out it does not contain sufficient information to ascer= tain +whether your builder had committed to the level of quality you were expect= ing. + +Similar situations happen in code reviews very frequently and can lead to = a long +discussion before it can be resolved. The most important thing is to +**identify** a disagreement as such early and then call it out. Tips on ho= w to +do this, can be found [here][8]. + +At this point, you will understand why you have the disagreement, but not +necessarily agreement on how to move forward. An easy fix would be to agre= e to +submit the change as it is and fix it in future. In a corporate software +engineering environment this is the most likely outcome, but in open source +communities additional concerns have to be considered. +* Code reviewers frequently have been in this situation before with the mo= st + common outcome that the issue is then never fixed. By accepting the chan= ge, + the reviewers have no leverage to fix the issue and may have to spend ef= fort + fixing the issue themselves in future as it may impact the product they = built + on top of the code. +* Conversely, a reviewer may be asking the author to make too many changes= of + this type which ultimately may lead the author to not contribute to the + project again. +* An author, which consistently does not address **any** of these issues m= ay + end up getting a bad reputation and may find future code reviews more + difficult. +* An author which always addresses **all** of these issues may end up gett= ing + into difficulties with their employer, as they are too slow getting code + upstreamed. + +None of these outcomes are good, so ultimately a balance has to be found. = At +the end of the day, the solution should focus on what is best for the comm= unity, +which may mean asking for an independent opinion as outlined in the next +section. + +## Issue: Multiple ways to solve a problem + +Frequently it is possible that a problem can be solved in multiple ways an= d it +is not always obvious which one is best. Code reviewers tend to follow the= ir +personal coding style when reviewing cide and sometimes will suggest that a +code author makes changes to follow their own style, even when the author's +code is correct. In such cases, it is easy to disagree and start arguing. + +We recommend that the code author tries to follow the code reviewers reque= sts, +even if they could be considered style issues, trusting the experience of= the +code reviewer. Similarly, we ask code reviewers to let the contributor hav= e the +freedom of implementation choices, where they do not have a downside. + +We do not always succeed in this, as such it is important to **identify** = such a +situation and then call it out as outlined [here][8]. + +## Resolution: Asking for an independent opinion + +Most disagreements can be settled by +* Asking another maintainer or committer to provide an independent opinion= on the + specific issue in public to help resolve it +* Failing this an issue can be escalated to the project leadership team, w= hich is + expected to act as referee and make a decision on behalf of the community + +If you feel uncomfortable with this approach, you may also contact +mediation@xenproject.org to get advice. See our [Communication Guide][9] +for more information. + +## Decision making and conflict resolution in our governance + +Our [governance][A] contains several proven mechanisms to help with decisi= on +making and conflict resolution. + +See +* [Expressing agreement and disagreement][B] +* [Lazy consensus / Lazy voting][7] +* [Informal votes or surveys][6] +* [Leadership team decisions][C] +* [Conflict resolution][D] + +[1]: http://www.paulgraham.com/disagree.html +[2]: https://upload.wikimedia.org/wikipedia/commons/a/a3/Graham%27s_Hierar= chy_of_Disagreement-en.svg +[3]: http://www.createdebate.com/user/viewprofile/Loudacris +[4]: https://en.wikipedia.org/wiki/User:Rocket000 +[5]: https://en.wiktionary.org/wiki/bikeshedding +[6]: https://xenproject.org/developers/governance/#informal-votes-or-surve= ys +[7]: https://xenproject.org/developers/governance/#lazyconsensus +[8]: communication-practice.md#Misunderstandings +[9]: communication-guide.md +[A]: https://xenproject.org/developers/governance/#decisions +[B]: https://xenproject.org/developers/governance/#expressingopinion +[C]: https://xenproject.org/developers/governance/#leadership +[D]: https://xenproject.org/developers/governance/#conflict --=20 2.13.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel