From patchwork Thu Sep 26 19:39:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lars Kurth X-Patchwork-Id: 11163725 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 65F3718A6 for ; Fri, 27 Sep 2019 04:20:15 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4037020863 for ; Fri, 27 Sep 2019 04:20:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4037020863 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=xenproject.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDhiO-0003E6-V1; Fri, 27 Sep 2019 04:18:44 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDZcj-0001N6-My for xen-devel@lists.xenproject.org; Thu, 26 Sep 2019 19:40:21 +0000 X-Inumbo-ID: 624bdf52-e095-11e9-965e-12813bfff9fa Received: from mail.xenproject.org (unknown [104.130.215.37]) by localhost (Halon) with ESMTPS id 624bdf52-e095-11e9-965e-12813bfff9fa; Thu, 26 Sep 2019 19:39:41 +0000 (UTC) Received: from xenbits.xenproject.org ([104.239.192.120]) by mail.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDZbz-0002H2-GV; Thu, 26 Sep 2019 19:39:35 +0000 Received: from localhost ([127.0.0.1] helo=MacBook-Pro-2.Home) by xenbits.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDZbz-0007uS-Ac; Thu, 26 Sep 2019 19:39:35 +0000 From: Lars Kurth To: xen-devel@lists.xenproject.org Date: Thu, 26 Sep 2019 20:39:24 +0100 Message-Id: <2e4b36afaa73277d246d7e84037db1532a136ec7.1569525222.git.lars.kurth@citrix.com> X-Mailer: git-send-email 2.13.0 In-Reply-To: References: In-Reply-To: References: X-Mailman-Approved-At: Fri, 27 Sep 2019 04:18:43 +0000 Subject: [Xen-devel] [PATCH v2 6/6] 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 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 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 | 146 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 resolving-disagreement.md diff --git a/resolving-disagreement.md b/resolving-disagreement.md new file mode 100644 index 0000000..19aedbe --- /dev/null +++ b/resolving-disagreement.md @@ -0,0 +1,146 @@ +# 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](http://www.paulgraham.com/disagree.html)**, putting types of +arguments into a seven-point hierarchy and observing that *moving up the +disagreement hierarchy makes people 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 disagreement are rarer. + +| ![Graham's Hierarchy of Disagreemen](https://upload.wikimedia.org/wikipedia/commons/a/a3/Graham%27s_Hierarchy_of_Disagreement-en.svg) | +| *A representation of Graham's hierarchy of disagreement from [Loudacris](http://www.createdebate.com/user/viewprofile/Loudacris) modified by [Rocket000](https://en.wikipedia.org/wiki/User:Rocket000)* | + +In the context of the Xen Project we strive to **only use the top half** of 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 reviewer +asks or appears to ask the author of patch to implement additional functionality. + +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 reviewer. To resolve +this, it is advisable to +* Report your findings +* And then check whether this was merely an interesting suggestion, or something 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 implemented +* 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 get an +independent opinion from another maintainer or the project's leadership team. + +## Issue: [Bikeshedding](https://en.wiktionary.org/wiki/bikeshedding) + +Occasionally discussions about unimportant but easy-to-grasp issues can lead to +prolonged and unproductive discussion. The best way to approach this is to +try and **anticipate** bikeshedding and highlight it as such upfront. However, the +format of a code review does not always lend itself well to this approach, except +for highlighting it in the cover letter of a patch series. + +However, typically Bikeshedding issues are fairly easy to recognize in a code review, +as you will very quickly get different reviewers providing differing 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](https://xenproject.org/developers/governance/#informal-votes-or-surveys) or +[lazy voting](https://xenproject.org/developers/governance/#lazyconsensus) 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 have to be resolved or +not before the code is ready to be submitted. Such disagreements are typically caused +by different expectations related to the level of perfection a patch series needs to fulfil +before it can be considered ready to be committed. + +To explain this better, I am going to use the analogy of some building work 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 event +* 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, which in the example of +the shower mixer may require significant re-work and potentially push-back from your +builder. You may have to refer to the initial statement of work, but it turns out it does +not contain sufficient information to ascertain whether your builder had committed to +the level of quality you were expecting. + +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 how to do this, can be found +[here](communication-practice.md#Misunderstandings). + +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 agree 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 most common + outcome that the issue is then never fixed. By accepting the change, the reviewers + have no leverage to fix the issue and may have to spend effort 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 may 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 getting into + difficulties with their employer, as they are too slow getting code upstreamed. + +None of these outcomes are good, so ultimately a balance has been found. At the end +of the day, the solution should focus on what is best for the community, which may +mean asking for an independent opinion as outlined in the next section. + +## 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, which 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](communication-guide.md) +for more information. + +## Decision making and conflict resolution in our governance + +Our [governance](https://xenproject.org/developers/governance/#decisions) contains +several proven mechanisms to help with decision making and conflict resolution. + +See +* [Expressing agreement and disagreement](https://xenproject.org/developers/governance/#expressingopinion) +* [Lazy consensus / Lazy voting](https://xenproject.org/developers/governance/#lazyconsensus) +* [Informal votes or surveys](https://xenproject.org/developers/governance/#informal-votes-or-surveys) +* [Leadership team decisions](https://xenproject.org/developers/governance/#leadership) +* [Conflict resolution](https://xenproject.org/developers/governance/#conflict)