[win-pv-devel] [PATCH v4 7/7] Added Resolving Disagreement

[Lars Kurth <lars.kurth@xxxxxxxxxxxxxx>]

From: Lars Kurth <lars.kurth@xxxxxxxxxx>

This guide provides Best Practice on identifying and resolving
common classes of disagreement

Changes since v3
* Fixed broken http link (typo)

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 <lars.kurth@xxxxxxxxxx>
--
Cc: minios-devel@xxxxxxxxxxxxxxxxxxxx
Cc: xen-api@xxxxxxxxxxxxxxxxxxxx
Cc: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
Cc: mirageos-devel@xxxxxxxxxxxxxxxxxxxx
Cc: committers@xxxxxxxxxxxxxx
---
 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..fb3b134
--- /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 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 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** 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 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 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][5]
+
+Occasionally discussions about unimportant but easy-to-grasp issues can lead to
+prolonged and unproductive discussions. 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][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 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 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, 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][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 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 to be 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.
+
+## Issue: Multiple ways to solve a problem
+
+Frequently it is possible that a problem can be solved in multiple ways and it
+is not always obvious which one is best. Code reviewers tend to follow their
+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 requests,
+even  if they could be considered style issues, trusting the experience of the
+code reviewer. Similarly, we ask code reviewers to let the contributor have 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, 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@xxxxxxxxxxxxxx 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 decision
+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_Hierarchy_of_Disagreement-en.svg
+[3]: https://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-surveys
+[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
-- 
2.13.0


_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel