+++ /dev/null
-# Xen Project Code of Conduct
-
-## Our Pledge
-
-In the interest of fostering an open and welcoming environment, we as
-contributors and maintainers pledge to make participation in our project and
-our community a harassment-free experience for everyone, regardless of age, body
-size, disability, ethnicity, sex characteristics, gender identity and
-expression, level of experience, education, socio-economic status, nationality,
-personal appearance, race, religion, or sexual identity and orientation.
-
-## Our Standards
-
-We believe that a Code of Conduct can help create a harassment-free environment,
-but is not sufficient to create a welcoming environment on its own: guidance on
-creating a welcoming environment, how to communicate in an effective and
-friendly way, etc. can be found [here][guidance]].
-
-Examples of unacceptable behavior by participants include:
-
-* The use of sexualized language or imagery and unwelcome sexual attention or
- advances
-* Trolling, insulting/derogatory comments, and personal or political attacks
-* Public or private harassment
-* Publishing others' private information, such as a physical or electronic
- address, without explicit permission
-* Other conduct which could reasonably be considered inappropriate in a
- professional setting
-
-## Our Responsibilities
-
-Project leadership team members are responsible for clarifying the standards of
-acceptable behavior and are expected to take appropriate and fair corrective
-action in response to any instances of unacceptable behavior.
-
-Project leadership team members have the right and responsibility to remove,
-edit, or reject comments, commits, code, wiki edits, issues, and other
-contributions that are not aligned to this Code of Conduct, or to ban
-temporarily or permanently any contributor for other behaviors that they deem
-inappropriate, threatening, offensive, or harmful.
-
-## Scope
-
-This Code of Conduct applies within all project spaces of all sub-projects,
-and it also applies when an individual is representing the project or its
-community in public spaces. Examples of representing a project or community
-include using an official project e-mail address, posting via an official social
-media account, or acting as an appointed representative at an online or offline
-event. Representation of a project may be further defined and clarified by the
-project leadership.
-
-## What to do if you witness or are subject to unacceptable behavior
-
-Instances of abusive, harassing, or otherwise unacceptable behavior may be
-reported by contacting Conduct Team members at conduct@xenproject.org. All
-complaints will be reviewed and investigated and will result in a response that
-is deemed necessary and appropriate to the circumstances. Conduct Team members
-are obligated to maintain confidentiality with regard to the reporter of an
-incident. Further details of specific enforcement policies may be posted
-separately.
-
-If you have concerns about any of the members of the conduct@ alias,
-you are welcome to contact precisely the Conduct Team member(s) of
-your choice.
-
-Project leadership team members who do not follow or enforce the Code of Conduct
-in good faith may face temporary or permanent repercussions as determined by
-other members of the project's leadership.
-
-## Conduct Team members
-Conduct Team members are project leadership team members from any
-sub-project. The current list of Conduct Team members is:
-* Lars Kurth <lars dot kurth at xenproject dot org>
-* George Dunlap <george dot dunlap at citrix dot com>
-* Ian Jackson <ian dot jackson at citrix dot com>
-
-Conduct Team members are changed by proposing a change to this document,
-posted on all sub-project lists, followed by a formal global vote as outlined
-[here]: https://xenproject.org/developers/governance/#project-decisions
-
-## Attribution
-
-This Code of Conduct is adapted from the [Contributor Covenant][homepage],
-version 1.4, available at
-https://www.contributor-covenant.org/version/1/4/code-of-conduct.html
-
-[homepage]: https://www.contributor-covenant.org
-[guidance]: communication-guide.md
-
-For answers to common questions about this code of conduct, see
-https://www.contributor-covenant.org/faq
+++ /dev/null
-# Code Review Guide
-
-This document highlights what reviewers such as maintainers and committers look
-for when reviewing your code. It sets expectations for code authors and provides
-a framework for code reviewers.
-
-Before we start, it is important to remember that the primary purpose of a
-a code review is to identify any bugs or potential bugs in the code. Most code
-reviews are relatively straight-forward and do not require re-writing the
-submitted code substantially.
-
-The document provides advice on how to structure larger patch series and
-provides pointers on code author's and reviewer's workflows.
-
-Sometimes it happens that a submitted patch series made wrong assumptions or has
-a flawed design or architecture. This can be frustrating for contributors and
-code reviewers. Note that this document does contain [a section](#problems)
-that provides suggestions on how to minimize the impact for most stake-holders
-and also on how to avoid such situations.
-
-This document does **not cover** the following topics:
-* [Communication Best Practice][1]
-* [Resolving Disagreement][2]
-* [Patch Submission Workflow][3]
-* [Managing Patch Submission with Git][4]
-
-## What we look for in Code Reviews
-
-When performing a code review, reviewers typically look for the following things
-
-### Is the change necessary to accomplish the goals?
-
-* Is it clear what the goals are?
-* Do we need to make a change, or can the goals be met with existing
- functionality?
-
-### Architecture / Interface
-
-* Is this the best way to solve the problem?
-* Is this the right part of the code to modify?
-* Is this the right level of abstraction?
-* Is the interface general enough? Too general? Forward compatible?
-
-### Functionality
-
-* Does it do what it’s trying to do?
-* Is it doing it in the most efficient way?
-* Does it handle all the corner / error cases correctly?
-
-### Maintainability / Robustness
-
-* Is the code clear? Appropriately commented?
-* Does it duplicate another piece of code?
-* Does the code make hidden assumptions?
-* Does it introduce sections which need to be kept **in sync** with other
- sections?
-* Are there other **traps** someone modifying this code might fall into?
-
-**Note:** Sometimes you will work in areas which have identified maintainability
-and/or robustness issues. In such cases, maintainers may ask you to make
-additional changes, such that your submitted code does not make things worse or
-point you to other patches are already being worked on.
-
-### System properties
-
-In some areas of the code, system properties such as
-* Code size
-* Performance
-* Scalability
-* Latency
-* Complexity
-* &c
-are also important during code reviews.
-
-### Style
-
-* Comments, carriage returns, **snuggly braces**, &c
-* See [CODING_STYLE][5] and [tools/libxl/CODING_STYLE][6]
-* No extraneous whitespace changes
-
-### Documentation and testing
-
-* If there is pre-existing documentation in the tree, such as man pages, design
- documents, etc. a contributor may be asked to update the documentation
- alongside the change. Documentation is typically present in the [docs][7]
- folder.
-* When adding new features that have an impact on the end-user,
- a contributor should include an update to the [SUPPORT.md][8] file.
- Typically, more complex features require several patch series before it is
- ready to be advertised in SUPPORT.md
-* When adding new features, a contributor may be asked to provide tests or
- ensure that existing tests pass
-
-#### Testing for the Xen Project Hypervisor
-
-Tests are typically located in one of the following directories
-* **Unit tests**: [tools/tests][9] or [xen/test][A]<br>
- Unit testing is hard for a system like Xen and typically requires building a
- subsystem of your tree. If your change can be easily unit tested, you should
- consider submitting tests with your patch.
-* **Build and smoke test**: see [Xen GitLab CI][B]<br>
- Runs build tests for a combination of various distros and compilers against
- changes committed to staging. Developers can join as members and test their
- development branches **before** submitting a patch.
-* **XTF tests** (microkernel-based tests): see [XTF][C]<br>
- XTF has been designed to test interactions between your software and hardware.
- It is a very useful tool for testing low level functionality and is executed
- as part of the project's CI system. XTF can be easily executed locally on
- xen.git trees.
-* **osstest**: see [README][D]<br>
- Osstest is the Xen Projects automated test system, which tests basic Xen use
- cases on a variety of different hardware. Before changes are committed, but
- **after** they have been reviewed. A contributor’s changes **cannot be
- applied to master** unless the tests pass this test suite. Note that XTF and
- other tests are also executed as part of osstest.
-
-### Patch / Patch series information
-
-* Informative one-line changelog
-* Full changelog
-* Motivation described
-* All important technical changes mentioned
-* Changes since previous revision listed
-* Reviewed-by’s and Acked-by’s dropped if appropriate
-
-More information related to these items can be found in our
-[Patch submission Guide][E].
-
-## Code Review Workflow
-
-This section is important for code authors and reviewers. We recommend that in
-particular new code authors carefully read this section.
-
-### Workflow from a Reviewer's Perspective
-
-Patch series typically contain multiple changes to the codebase, some
-transforming the same section of the codebase multiple times. It is quite common
-for patches in a patch series to rely on the previous ones. This means that code
-reviewers review patches and patch series **sequentially** and **the structure
-of a patch series guides the code review process**. Sometimes in a long series,
-patches {1,2}/10 will be clean-ups, {3-6}/10 will be general reorganizations
-which don't really seem to do anything and then {7-10}/10 will be the substance
-of the series, which helps the code reviewer understand what {3-6}/10 were
-about.
-
-Generally there are no hard rules on how to structure a series, as the structure
-of a series is very code specific and it is hard to give specific advice. There
-are some general tips which help and some general patterns.
-
-**Tips:**
-
-* Outline the thinking behind the structure of the patch series. This can make
- a huge difference and helps ensure that the code reviewer understands what the
- series is trying to achieve and which portions are addressing which problems.
-* Try and keep changes that belong to a subsystem together
-* Expect that the structure of a patch series sometimes may need to change
- between different versions of a patch series
-* **Most importantly**: Start small. Don't submit a large and complex patch
- series as the first interaction with the community. Try and pick a smaller
- task first (e.g. a bug-fix, a clean-up task, etc.) such that you don't have
- to learn the tools, code and deal with a large patch series all together for
- the first time.
-
-**General Patterns:**
-
-If there are multiple subsystems involved in your series, then these are best
-separated out into **sets of patches**, which roughly follow the following
-seven categories. In other words: you would end up with **7 categories x N
-subsystems**. In some cases, there is a **global set of patches** that affect
-all subsystems (e.g. headers, macros, documentation) impacting all changed
-subsystems which ideally comes **before** subsystem specific changes.
-
-The seven categories typically making up a logical set of patches
-1. Cleanups and/or new Independent Helper Functions
-2. Reorganizations
-3. Headers, APIs, Documentation and anything which helps understand the
- substance of a series
-4. The substance of the change
-5. Cleanups of any infelicities introduced temporarily
-6. Deleting old code
-7. Test code
-
-Note that in many cases, some of the listed categories are not always present
-in each set, as they are not needed. Of course, sometimes there are several
-patches describing **changes of substance**, which could be ordered in different
-ways: in such cases it may be necessary to put reorganizations in between these
-patches.
-
-If a series is structured this way, it is often possible to agree early on,
-that a significant portion of the changes are fine and to check these in
-independently of the rest of the patch series. This means that there is
-* Less work for authors to rebase
-* Less cognitive overhead for reviewers to review successive versions of a
- series
-* The possibility for different code reviewers to review portions of such
- large changes independently
-
-**Trade-Offs:**
-
-* In some cases, following the general pattern above may create extra patches
- and may make a series more complex and harder to understand.
-* Crafting a more extensive cover letter will be extra effort: in most cases,
- the extra time investment will be saving time during the code review process.
- Verbosity is not the goal, but clarity is. Before you send a larger series
- in particular: try and put yourself into the position of a code reviewer and
- try to identify information that helps a code reviewer follow the patch
- series.
-* In cases where changes need to be back-ported to older releases, moving
- general cleanups last is often preferable: in such cases the **substance of
- the change** is back-ported, whereas general cleanups and improvements are
- not.
-
-**Example:**
-* [[PATCH v3 00/18] VM forking][H] is a complex patch series with an exemplary
- cover letter. Notably, it contains the following elements
- * It provides a description of the design goals and detailed description
- of the steps required to fork a VM.
- * A description of changes to the user interface
- * It contains some information about the test status of the series including
- some performance information.
- * It maps the series onto the categories listed above. As expected, not
- all categories are used in this case. However, the series does contain
- elements of **1** (in this case preparation to enable the functionality),
- **2** reorganizations and other non-functional changes that enable the
- rest of the series and **4** the substance of the series with additional
- information to make it easier for the reviewer to parse the series.
-
-### Workflow from an Author's Perspective
-
-When code authors receive feedback on their patches, they typically first try
-to clarify feedback they do not understand. For smaller patches or patch series
-it makes sense to wait until receiving feedback on the entire series before
-sending out a new version addressing the changes. For larger series, it may
-make sense to send out a new revision earlier.
-
-As a reviewer, you need some system that helps ensure that you address all
-review comments. This can be tedious when trying to map a hierarchical e-mail
-thread onto a code-base. Different people use different techniques from using
-* In-code TODO statements with comment snippets copied into the code
-* To keeping a separate TODO list
-* To printing out the review conversation tree and ticking off what has been
- addressed
-* A combination of the above
-
-### <a name="problems"></a>Problematic Patch Reviews
-
-A typical waterfall software development process is sequential with the
-following steps: define requirements, analyze, design, code, test and deploy.
-Problems uncovered by code review or testing at such a late stage can cause
-costly redesign and delays. The principle of **[Shift Left][D]** is to take a
-task that is traditionally performed at a late stage in the process and perform
-that task at earlier stages. The goal is to save time by avoiding refactoring.
-
-Typically, problematic patch reviews uncover issues such as wrong or missed
-assumptions, a problematic architecture or design, or other bugs that require
-significant re-implementation of a patch series to fix the issue.
-
-The principle of **Shift Left** also applies in code reviews. Let's assume a
-series has a major flaw: ideally, this flaw would be picked up in the **first
-or second iteration** of the code review. As significant parts of the code may
-have to be re-written, it does not make sense for reviewers to highlight minor
-issues (such as style issues) until major flaws have been addressed of the
-affected part of a patch series. In such cases, providing feedback on minor
-issues reviewers cause the code author and themselves extra work by asking for
-changes to code, which ultimately may be changed later.
-
-To make it possible for code reviewers to identify major issues early, it is
-important for code-authors to highlight possible issues in a cover letter and
-to structure a patch series in such a way that makes it easy for reviewers to
-separate difficult and easy portions of a patch series. This will enable
-reviewers to progress uncontroversial portions of a patch independently from
-controversial ones.
-
-### Reviewing for Patch Authors
-
-The following presentation by George Dunlap, provides an excellent overview on
-how we do code reviews, specifically targeting non-maintainers.
-
-As a community, we would love to have more help reviewing, including from **new
-community members**. But many people
-* do not know where to start, or
-* believe that their review would not contribute much, or
-* may feel intimidated reviewing the code of more established community members
-
-The presentation demonstrates that you do not need to worry about any of these
-concerns. In addition, reviewing other people's patches helps you
-* write better patches and experience the code review process from the other
- side
-* and build more influence within the community over time
-
-Thus, we recommend strongly that **patch authors** read the watch the recording
-or read the slides:
-* [Patch Review for Non-Maintainers slides][F]
-* [Patch Review for Non-Maintainers recording - 20"][G]
-
-[1]: communication-practice.md
-[2]: resolving-disagreement.md
-[3]: https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches
-[4]: https://wiki.xenproject.org/wiki/Managing_Xen_Patches_with_Git
-[5]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE
-[6]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/CODING_STYLE
-[7]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=docs
-[8]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=SUPPORT.md
-[9]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=tools/tests
-[A]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=xen/test
-[B]: https://gitlab.com/xen-project/xen/pipelines
-[C]: https://xenbits.xenproject.org/docs/xtf/
-[D]: https://xenbits.xenproject.org/gitweb/?p=osstest.git;a=blob;f=README
-[E]: https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches
-[D]: https://devopedia.org/shift-left
-[F]: https://www.slideshare.net/xen_com_mgr/xpdds19-keynote-patch-review-for-nonmaintainers-george-dunlap-citrix-systems-uk-ltd
-[G]: https://www.youtube.com/watch?v=ehZvBmrLRwg
-[H]: https://lists.xenproject.org/archives/html/xen-devel/2019-12/threads.html#02097
+++ /dev/null
-# Communication Guide
-
-We believe that our [Code of Conduct](code-of-conduct.md) can help create a
-harassment-free environment, but is not sufficient to create a welcoming
-environment on its own. We can all make mistakes: when we do, we take
-responsibility for them and try to improve.
-
-This document lays out our gold standard, best practices for some common
-situations and mechanisms to help resolve issues that can have a
-negative effect on our community.
-
-## Goal
-
-We want a productive, welcoming and agile community that can welcome new
-ideas in a complex technical field which is able to reflect on and improve how
-we work.
-
-## Communication & Handling Differences in Opinions
-
-Examples of behavior that contributes to creating a positive environment
-include:
-* Use welcoming and inclusive language
-* Keep discussions technical and actionable
-* Be respectful of differing viewpoints and experiences
-* Be aware of your own and counterpart’s communication style and culture
-* Gracefully accept constructive criticism
-* Focus on what is best for the community
-* Show empathy towards other community members
-* Resolve differences in opinion effectively
-
-## Getting Help
-
-When developing code collaboratively, technical discussion and disagreements
-are unavoidable. Our contributors come from different countries and cultures,
-are driven by different goals and take pride in their work and in their point
-of view. This invariably can lead to lengthy and unproductive debate,
-followed by indecision, sometimes this can impact working relationships
-or lead to other issues that can have a negative effect on our community.
-
-To minimize such issue, we provide a 3-stage process
-* Self-help as outlined in this document
-* Ability to ask for an independent opinion or help in private
-* Mediation between parties which disagree. In this case a neutral community
- member assists the disputing parties resolve the issues or will work with the
- parties such that they can improve future interactions.
-
-If you need and independent opinion or help, feel free to contact
-mediation@xenproject.org. The team behind mediation@ is made up of the
-same community members as those listed in the Conduct Team: see
-[Code of Conduct](code-of-conduct.md). In addition, team members are obligated
-to maintain confidentiality with regard discussions that take place. If you
-have concerns about any of the members of the mediation@ alias, you are
-welcome to contact precisely the team member(s) of your choice. In this case,
-please make certain that you highlight the nature of a request by making sure
-that either help or mediation is mentioned in the e-mail subject or body.
-
-## Specific Topics and Best Practice
-
-* [Code Review Guide](code-review-guide.md):
- Essential reading for code reviewers and contributors
-* [Communication Best Practice](communication-practice.md):
- This guide covers communication guidelines for code reviewers and authors.
- It should help you create self-awareness, anticipate, avoid and help resolve
- communication issues.
-* [Resolving Disagreement](resolving-disagreement.md):
- This guide lays out common situations that can lead to dead-lock and shows
- common patterns on how to avoid and resolve issues.
+++ /dev/null
-# Communication Best Practice
-
-This guide provides communication Best Practice that helps you in
-* Using welcoming and inclusive language
-* Keeping discussions technical and actionable
-* Being respectful of differing viewpoints and experiences
-* Being aware of your own and counterpart’s communication style and culture
-* Show empathy towards other community members
-
-## Code reviews for **reviewers** and **patch authors**
-
-Before embarking on a code review, it is important to remember that
-* A poorly executed code review can hurt the contributors feeling, even when a
- reviewer did not intend to do so. Feeling defensive is a normal reaction to
- a critique or feedback. A reviewer should be aware of how the pitch, tone,
- or sentiment of their comments could be interpreted by the contributor. The
- same applies to responses of an author to the reviewer.
-* When reviewing someone's code, you are ultimately looking for issues. A good
- code reviewer is able to mentally separate finding issues from articulating
- code review comments in a constructive and positive manner: depending on your
- personality this can be **difficult** and you may need to develop a technique
- that works for you.
-* As software engineers we like to be proud of the solutions we came up with.
- This can make it easy to take another people’s criticism personally. Always
- remember that it is the code that is being reviewed, not you as a person.
-* When you receive code review feedback, please be aware that we have reviewers
- from different backgrounds, communication styles and cultures. Although we
- are all trying to create a productive, welcoming and agile environment, we do
- not always succeed.
-
-### Express appreciation
-
-As the nature of code review to find bugs and possible issues, it is very easy
-for reviewers to get into a mode of operation where the patch review ends up
-being a list of issues, not mentioning what is right and well done. This can
-lead to the code submitter interpreting your feedback in a negative way.
-
-The opening of a code review provides an opportunity to address this and also
-sets the tone for the rest of the code review. Starting **every** review on a
-positive note, helps set the tone for the rest of the review.
-
-For an initial patch, you can use phrases such as
-> Thanks for the patch
-> Thanks for doing this
-
-For further revisions within a review, phrases such as
-> Thank you for addressing the last set of changes
-
-If you believe the code was good, it is good practice to highlight this by
-using phrases such as
-> Looks good, just a few comments
-> The changes you have made since the last version look good
-
-If you think there were issues too many with the code to use one of the
-phrases, you can still start on a positive note, by for example saying
-> I think this is a good change
-> I think this is a good feature proposal
-
-It is also entirely fine to highlight specific changes as good. The best place
-to do this, is at the top of a patch, as addressing code review comments
-typically requires a contributor to go through the list of things to address
-and an in-lined positive comment is likely to break that workflow.
-
-You should also consider, that if you review a patch of an experienced
-contributor phrases such as *Thanks for the patch* could come across as
-patronizing, while using *Thanks for doing this* is less likely to be
-interpreted as such.
-
-Appreciation should also be expressed by patch authors when asking for
-clarifications to a review or responding to questions. A simple
-> Thank you for your feedback
-> Thank you for your reply
-> Thank you XXX!
-
-is normally sufficient.
-
-### Avoid inflammatory and negatively charged language
-
-The way a reviewer expresses feedback, has a big impact on how the author
-perceives the feedback. Choosing negatively charged language such as your code
-is terrible, fragile, spaghetti, inefficient, racy, messy, etc. creates a
-negative emotional response in the submitter, which can then make subsequent
-communication difficult. The same is true when a patch author is responding to
-a comment from a reviewer.
-
-One of our maintainers has been studying Mandarin for several years and has
-come across the most strongly-worded dictionary entry [he has ever seen][1].
-This example illustrates the differences between an inflammatory and fact-based
-description extremely well.
-
-> 裹脚 (guo3 jiao3): foot-binding (a vile feudal practice which crippled women
-> both physically and spiritually)
-
-This is not something one is used to hearing from dictionary entries. Once you
-investigate the practice foot-binding, it is hard to disagree with the
-dictionary entry. However, the statement does not contain much information. If
-you read it without knowing what foot-binding is, it is hard to be convinced
-by this statement. The main take-away is that the author of the dictionary
-entry had strong opinions about this topic. It does not tell you why you
-should have the same opinion.
-
-Compare this to the [Wikipedia entry][2]
-
-> Foot binding was the custom of applying tight binding to the feet of young
-> girls to modify the shape and size of their feet. ... foot binding was a
-> painful practice and significantly limited the mobility of women, resulting
-> in lifelong disabilities for most of its subjects. ... Binding usually
-> started during the winter months since the feet were more likely to be numb,
-> and therefore the pain would not be as extreme. …The toes on each foot
-> were curled under, then pressed with great force downwards and squeezed
-> into the sole of the foot until [redacted] ...
-
-Without going into the details of foot-binding, it is noticeable that the
-definition is a list of simple facts that are laid out in a way that make it
-obvious what the correct conclusion is.
-
-Because the Wikipedia entry is entirely fact based it is more powerful and
-persuasive than the dictionary entry. The same applies to code reviews.
-
-Making statements in code reviews such as
-> Your code is garbage
-> This idea is stupid
-
-besides negatively charged, rude and counter productive
-* It will make the patch author angry: instead of finding a solution to the
- problem the author will spend time and mental energy wrestling with their
- feelings
-* It does not contain any information
-* Facts are both more powerful and more persuasive
-
-Consider the following two pieces of feedback on a piece of code
-> This piece of code is confusing
-> It took me a long time to figure out what was going on here
-
-The first example expresses an opinion, whereas the second re-phrases the
-statement in terms of what you experienced, which is a fact.
-
-Other examples:
-> BAD: This is fragile
-> SOMEWHAT BETTER: This seems fragile to me
-> BEST: If X happens, Y will happen.
-
-A certain piece of code can be written in many different ways: this can lead to
-disagreements on the best architecture, design or coding pattern. As already
-pointed out in this section: avoid feedback that is opinion-based and thus
-does not add any value. Back your criticism (or idea on how to solve a
-problem) with a sensible rationale.
-
-### Review the code, not the person
-
-Without realizing it, it is easy to overlook the difference between insightful
-critique of code and personal criticism. Let's look at a theoretical function
-where there is an opportunity to return out of the function early. In this
-case, you could say
-
-> You should return from this function early, because of XXX
-
-On its own, there is nothing wrong with this statement. However, a code review
-is made up of multiple comments and using **You should** consistently can
-start to feel negative and can be mis-interpreted as a personal attack. Using
-something like avoids this issue:
-
-> Returning from this function early is better, because of XXX
-
-Without personal reference, a code review will communicate the problem, idea
-or issue without risking mis-interpretation.
-
-### Verbose vs. terse
-
-Due to the time it takes to review and compose code reviewer, reviewers often
-adopt a terse style. It is not unusual to see review comments such as
-> typo
-> s/resions/regions/
-> coding style
-> coding style: brackets not needed
-etc.
-
-Terse code review style has its place and can be productive for both the
-reviewer and the author. However, overuse can come across as unfriendly,
-lacking empathy and can thus create a negative impression with the author of a
-patch. This is in particular true, when you do not know the author or the
-author is a newcomer. Terse communication styles can also be perceived as rude
-in some cultures.
-
-If you tend to use a terse commenting style and you do not know whether the
-author is OK with it, it is often a good idea to compensate for it in the code
-review opening (where you express appreciation) or when there is a need for
-verbose expression. However, when you know are working with a seasoned code
-author, it is also entirely acceptable to drop niceties such as expressing
-appreciation with the goal to save the author and reviewer time.
-
-It is also entirely fine to mention that you have a fairly terse communication
-style and ask whether the author is OK with it. In almost all cases, they will
-be: by asking you are showing empathy that helps counteract a negative
-impression.
-
-### Clarity over verbosity
-
-When reading this document, you may get the impression that following the
-guidance outlined here takes more effort and time for both code reviewers and
-code authors. This is not the intention: much of this document aims to create
-clearer communication, which ultimately saves time by reducing unnecessary
-iterations during communication. We value **clarity over verbosity**.
-
-Areas which often create unnecessary back-and-forth between reviewers and
-authors are
-* Unstated assumptions and goals
-* Leave suggestions, examples, and resources (such as links to existing code)
-* There is nothing more helpful for the thought process than example. It
- guarantees that you have a shared understanding and reduces the questions
- asked on a comment.
-
-### Code Review Comments should be actionable
-
-Code review comments should be actionable: in other words, it needs to be clear
-what the author of the code needs to do to address the issue you identified.
-
-Statements such as
-> BAD: This is wrong
-> BAD: This does not work
-> BETTER, BUT NOT GOOD: This does not work, because of XXX
-
-do not normally provide the author of a patch with enough information to send
-out a new patch version. By doing this, you essentially force the patch author
-to **find** and **implement** an alternative, which then may also not be
-acceptable to you as the **reviewer** of the patch.
-
-A better way to approach this is to say
-
-> This does not work, because of XXX
-> You may want to investigate YYY and ZZZ as alternatives
-
-In some cases, it may not be clear whether YYY or ZZZ are the better solution.
-As a reviewer you should be as up-front and possible in such a case and say
-something like
-
-> I am not sure whether YYY and ZZZ are better, so you may want to outline your
-> thoughts about both solutions by e-mail first, such that we can decide what
-> works best
-
-### Identify the severity and optionality of review comments
-
-By default, every comment which is made **ought to be addressed** by the
-author. However, sometimes reviewers note issues, which would be nice if they
-were addressed, but are not mandatory to fix.
-
-Typically, reviewers use terminology such as
-> This would be a nice-to-have
-> This is not a blocker
-
-Some maintainers use
-> NIT: XXX
-
-however, it is sometimes also used to indicate a minor issue that **must** be
-fixed. Also terminology such as **this is not a blocker** could be
-misinterpreted. It is important that **reviewers** use language that make
-clear whether a comment is an optional suggestion. Examples may be
-> NIT (optional): XXX
-> I think it would be good if X also did Y, not a requirement but nice-to-have
-
-### Identify the severity of a disagreement
-
-During a code review, it can happen that reviewer and author disagree on how
-to move forward. The default position when it comes to disagreements is that
-**both parties want to argue their case**. However, frequently one or both
-parties do not feel that strongly about a specific issue.
-
-Within the Xen Project, we have [a way][3] to highlight one's position on
-proposals, formal or informal votes using the following notation:
-> +2 : I am happy with this proposal, and I will argue for it
-> +1 : I am happy with this proposal, but will not argue for it
-> 0 : I have no opinion
-> -1 : I am not happy with this proposal, but will not argue against it
-> -2 : I am not happy with this proposal, and I will argue against it
-
-You can use a phrase such as
-> I am not happy with this suggestion, but will not argue against it
-
-to make clear where you stand, while recording your position. Conversely, a
-reviewer may do something similar
-> I am not happy with XYZ, but will not argue against it [anymore]
-> What we have now is good enough, but could be better
-
-### Authors: responding to review comments
-
-Typically patch authors are expected to **address all** review comments in the
-next version of a patch or patch series. In a smooth-running code review where
-you do not have further questions it is not at all necessary to acknowledge
-the changes you are going to make:
-* Simply send the next version with the changes addressed and record it in the
- change-log
-
-When there is discussion, the normal practice is to remove the portion of the
-e-mail thread where there is agreement. Otherwise, the thread can become
-exceptionally long.
-
-In cases where there was discussion and maybe disagreement, it does however
-make sense to close the discussion by saying something like
-
-> ACK
-> Seems we are agreed, I am going to do this
-
-Other situations when you may want to do this are cases where the reviewer made
-optional suggestions, to make clear whether the suggestion will be followed or
-not.
-
-### Avoid uncommon words: not everyone is a native English speaker
-
-Avoid uncommon words both when reviewing code or responding to a review. Not
-everyone is a native English speaker. The use of such words can come across
-badly and can lead to misunderstandings.
-
-### Prioritize significant flaws
-
-If a patch or patch series has significant flaws, such as
-* It is built on wrong assumptions
-* There are issues with the architecture or the design
-
-it does not make sense to do a detailed code review. In such cases, it is best
-to focus on the major issues first and deal with style and minor issues in a
-subsequent review. Not all series have significant flaws, but most series have
-different classes of changes that are required for acceptance: covering a
-range of major code modifications to minor code style fixes. To avoid
-misunderstandings between reviewers and contributors, it is important to
-establish and agree whether a series or part of a series has a significant
-flaw and agree a course of action.
-
-A pragmatic approach would be to
-* Highlight problematic portions of a series in the cover letter
-* For the patch author and reviewer(s) to agree that for problematic to omit
- style and minor issues in the review, until the significant flaw is addressed
-
-This saves both the patch author and reviewer(s) time. Note that some
-background is covered in detail in [Problematic Patch Reviews][4].
-
-
-### Reviewers: Welcome newcomers
-
-When reviewing the first few patches of a newcomer to the project, you may want
-spend additional time and effort in your code review. This contributes to a
-more **positive experience**, which ultimately helps create a positive working
-relationship in the long term.
-
-When someone does their first code submission, they will not be familiar with
-**all** conventions in the project. A good approach is to
-* Welcome the newcomer
-* Offer to help with specific questions, for example on IRC
-* Point to existing documentation: in particular if mistakes with the
- submission itself were made. In most situations, following the submission
- process makes the process more seamless for the contributor. So, you could
- say something like
-
-> Hi XXX. Welcome to the community and thank you for the patch
->
-> I noticed that the submission you made seems to not follow our process.
-> Are you aware of this document at YYY? If you follow the instructions the
-> entire code submission process and dealing with review comments becomes
-> much easier. Feel free to find me on IRC if you need specific help. My IRC
-> handle is ZZZ
-
-### Reviewers: Take account of previous reviewer(s) comments
-
-Sometimes multiple reviewers share reviewing a series. For example,
-reviewer John has reviewed the first 5 iterations of the series. The patch
-author has addressed all of John's comments and Susan comes in and picks up
-the series after iteration 5. In such cases it is possible that John and Susan
-have different styles, such as
-* different preferences on the code layout
-* different preferences on code style
-
-If Susan were to be strict on her own style and highlight her style
-preferences in subsequent reviews, this would cause additional re-work for the
-code author. In addition, it also causes extra work for Susan. The easiest way
-to avoid such situations, would be for Susan to focus on faulty code only and
-to disregard personal preferences when taking over the review of a series.
-
-### Reviewers: Review the code, then review the review
-
-As stated earlier it is often difficult to mentally separate finding issues
-from articulating code review comments in a constructive and positive manner.
-Even as an experienced code reviewer you can be in a bad mood, which can
- impact your communication style.
-
-A good trick to avoid this, is to start and complete the code review and then
-**not send it immediately**. You can then have a final go over the code review
-at some later point in time and review your comments from the other author's
-point of view. This minimizes the risk of being misunderstood. The same
-applies when replying to a code review: draft your reply and give it a final
-scan before pressing the send button.
-
-Generally, it is a good idea for code reviewers to do this regularly, purely
-from the viewpoint of self-improvement and self-awareness.
-
-## Common Communication Pitfalls
-
-This section contains common communication issues and provides suggestions on
-how to avoid them and resolve them. These are **general** issues which affect
-**all** online communication. As such, we can only try and do our best.
-
-### Misunderstandings
-
-When you meet face to face, you can read a person’s emotions. Even with a
-phone call, someone’s tone of voice can convey a lot of information. Using
-on-line communication channels you are flying blind, which often leads to
-misunderstandings. [Research][5] shows that in up to 50% of email
-conversations, the tone of voice is misinterpreted.
-
-In code reviews and technical discussions in general we tend to see two things
-* The reviewer or author interprets an exchange as too critical, passive
- aggressive, or other: this usually comes down to different cultures and
- communication styles, which are covered in the next section
-* There is an actual misunderstanding of a subject under discussion
-
-In the latter case, the key to resolution is to **identify the
-misunderstanding** as quickly as possible and call it out and de-escalate
-rather than let the misunderstanding linger. This is inherently difficult and
-requires more care than normal communication. Typically you would start with
-* Showing appreciation
-* Highlighting the potential misunderstanding and verifying whether the other
- person also feels that maybe there was a misunderstanding
-* Proposing a way forward: for example, it may make sense to move the
- conversation from the mailing list to [IRC][6] either in private or public,
- a community call or a private phone/video call.
-
-It is entirely acceptable to do this in a direct reply to your communication
-partner, rather than on a public e-mail list on or an otherwise public forum.
-
-A good approach is to use something like the following:
-> Hi XXX! Thank you for the insights you have given me in this code review
-> I feel that we are misunderstanding each other on the topic of YYY
-> Would you mind trying to resolve this on IRC. I am available at ZZZ
-
-Usually, technical misunderstandings come down two either
-1. Misinterpreting what the other person meant
-2. Different - usually unstated - assumptions on how something works or what
- is to be achieved
-3. Different - usually unstated - objectives and goals, which may be
- conflicting
-4. Real differences in opinion
-
-The goal of calling out a possible misunderstanding is to establish what
-caused the misunderstanding, such that all parties can move forward.
-Typically, 1 and 2 are easily resolved and will lead back to a constructive
-discussion. Whereas 3 and 4 may highlight an inherent disagreement, which may
-need to be resolved through techniques as
-outlined in [Resolving Disagreement][7].
-
-### Cultural differences and different communication styles
-
-The Xen Project is a global community with contributors from many different
-backgrounds. Typically, when we communicate with a person we know, we factor
-in past interactions. The less we know a person, the more we rely on cultural
-norms.
-
-However, different norms and value systems come into play when people from
-diverse cultural backgrounds interact. That can lead to misunderstandings,
-especially in sensitive situations such as conflict resolution, giving and
-receiving feedback, and consensus building.
-
-For example, giving direct feedback such as
-> [Please] replace XXX with YYY, as XXX does not do ZZZ
-
-is acceptable and normal in some cultures, whereas in cultures which value
-indirect feedback it would be considered rude. In the latter case, something
-like the following would be used
-> This looks very good to me, but I believe you should use YYY here,
-> because XXX would....
-
-The key to working and communicating well with people from different cultural
-backgrounds is **self-awareness**, which can then be used to either
-* Adapt your own communication style depending on who you talk to
-* Or to find a middle-ground that covers most bases
-
-A number of different theories in the field of working effectively are
-currently popular, with the most well-known one being
-[Erin Meyer's Culture Map][8]. A short overview can be found [here][9]
-(33 slides).
-
-### Code reviews and discussions are not competitions
-
-Code reviews on our mailing lists are not competitions on who can come up with
-the smartest solution or who is the real coding genius.
-
-In a code review - as well as in general - we expect that all stake-holders
-* Gracefully accept constructive criticism
-* Focus on what is best for the community
-* Resolve differences in opinion effectively
-
-The next section provides pointers on how to do this effectively.
-
-### Resolving Disagreement Effectively
-
-Common scenarios are covered our guide on [Resolving Disagreement][7], which
-lays out situations that can lead to dead-lock and shows common patterns on
-how to avoid and resolve issues.
-
-[1]: https://youtu.be/ehZvBmrLRwg?t=834
-[2]: https://en.wikipedia.org/wiki/Foot_binding
-[3]: https://xenproject.org/developers/governance/#expressingopinion
-[4]: resolving-disagreement.md#problems
-[5]: https://www.wired.com/2006/02/the-secret-cause-of-flame-wars/
-[6]: https://xenproject.org/help/irc/
-[7]: resolving-disagreement.md
-[8]: https://en.wikipedia.org/wiki/Erin_Meyer
-[9]: https://www.nsf.gov/attachments/134059/public/15LFW_WorkingWithMulticulturalTeams_LarsonC.pdf
+++ /dev/null
-# 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 fulfill 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 installment, 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 code 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@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 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
--- /dev/null
+# Xen Project Code of Conduct
+
+## Our Pledge
+
+In the interest of fostering an open and welcoming environment, we as
+contributors and maintainers pledge to make participation in our project and
+our community a harassment-free experience for everyone, regardless of age, body
+size, disability, ethnicity, sex characteristics, gender identity and
+expression, level of experience, education, socio-economic status, nationality,
+personal appearance, race, religion, or sexual identity and orientation.
+
+## Our Standards
+
+We believe that a Code of Conduct can help create a harassment-free environment,
+but is not sufficient to create a welcoming environment on its own: guidance on
+creating a welcoming environment, how to communicate in an effective and
+friendly way, etc. can be found [here][guidance]].
+
+Examples of unacceptable behavior by participants include:
+
+* The use of sexualized language or imagery and unwelcome sexual attention or
+ advances
+* Trolling, insulting/derogatory comments, and personal or political attacks
+* Public or private harassment
+* Publishing others' private information, such as a physical or electronic
+ address, without explicit permission
+* Other conduct which could reasonably be considered inappropriate in a
+ professional setting
+
+## Our Responsibilities
+
+Project leadership team members are responsible for clarifying the standards of
+acceptable behavior and are expected to take appropriate and fair corrective
+action in response to any instances of unacceptable behavior.
+
+Project leadership team members have the right and responsibility to remove,
+edit, or reject comments, commits, code, wiki edits, issues, and other
+contributions that are not aligned to this Code of Conduct, or to ban
+temporarily or permanently any contributor for other behaviors that they deem
+inappropriate, threatening, offensive, or harmful.
+
+## Scope
+
+This Code of Conduct applies within all project spaces of all sub-projects,
+and it also applies when an individual is representing the project or its
+community in public spaces. Examples of representing a project or community
+include using an official project e-mail address, posting via an official social
+media account, or acting as an appointed representative at an online or offline
+event. Representation of a project may be further defined and clarified by the
+project leadership.
+
+## What to do if you witness or are subject to unacceptable behavior
+
+Instances of abusive, harassing, or otherwise unacceptable behavior may be
+reported by contacting Conduct Team members at conduct@xenproject.org. All
+complaints will be reviewed and investigated and will result in a response that
+is deemed necessary and appropriate to the circumstances. Conduct Team members
+are obligated to maintain confidentiality with regard to the reporter of an
+incident. Further details of specific enforcement policies may be posted
+separately.
+
+If you have concerns about any of the members of the conduct@ alias,
+you are welcome to contact precisely the Conduct Team member(s) of
+your choice.
+
+Project leadership team members who do not follow or enforce the Code of Conduct
+in good faith may face temporary or permanent repercussions as determined by
+other members of the project's leadership.
+
+## Conduct Team members
+Conduct Team members are project leadership team members from any
+sub-project. The current list of Conduct Team members is:
+* Lars Kurth <lars dot kurth at xenproject dot org>
+* George Dunlap <george dot dunlap at citrix dot com>
+* Ian Jackson <ian dot jackson at citrix dot com>
+
+Conduct Team members are changed by proposing a change to this document,
+posted on all sub-project lists, followed by a formal global vote as outlined
+[here]: https://xenproject.org/developers/governance/#project-decisions
+
+## Attribution
+
+This Code of Conduct is adapted from the [Contributor Covenant][homepage],
+version 1.4, available at
+https://www.contributor-covenant.org/version/1/4/code-of-conduct.html
+
+[homepage]: https://www.contributor-covenant.org
+[guidance]: communication-guide.md
+
+For answers to common questions about this code of conduct, see
+https://www.contributor-covenant.org/faq
--- /dev/null
+# Code Review Guide
+
+This document highlights what reviewers such as maintainers and committers look
+for when reviewing your code. It sets expectations for code authors and provides
+a framework for code reviewers.
+
+Before we start, it is important to remember that the primary purpose of a
+a code review is to identify any bugs or potential bugs in the code. Most code
+reviews are relatively straight-forward and do not require re-writing the
+submitted code substantially.
+
+The document provides advice on how to structure larger patch series and
+provides pointers on code author's and reviewer's workflows.
+
+Sometimes it happens that a submitted patch series made wrong assumptions or has
+a flawed design or architecture. This can be frustrating for contributors and
+code reviewers. Note that this document does contain [a section](#problems)
+that provides suggestions on how to minimize the impact for most stake-holders
+and also on how to avoid such situations.
+
+This document does **not cover** the following topics:
+* [Communication Best Practice][1]
+* [Resolving Disagreement][2]
+* [Patch Submission Workflow][3]
+* [Managing Patch Submission with Git][4]
+
+## What we look for in Code Reviews
+
+When performing a code review, reviewers typically look for the following things
+
+### Is the change necessary to accomplish the goals?
+
+* Is it clear what the goals are?
+* Do we need to make a change, or can the goals be met with existing
+ functionality?
+
+### Architecture / Interface
+
+* Is this the best way to solve the problem?
+* Is this the right part of the code to modify?
+* Is this the right level of abstraction?
+* Is the interface general enough? Too general? Forward compatible?
+
+### Functionality
+
+* Does it do what it’s trying to do?
+* Is it doing it in the most efficient way?
+* Does it handle all the corner / error cases correctly?
+
+### Maintainability / Robustness
+
+* Is the code clear? Appropriately commented?
+* Does it duplicate another piece of code?
+* Does the code make hidden assumptions?
+* Does it introduce sections which need to be kept **in sync** with other
+ sections?
+* Are there other **traps** someone modifying this code might fall into?
+
+**Note:** Sometimes you will work in areas which have identified maintainability
+and/or robustness issues. In such cases, maintainers may ask you to make
+additional changes, such that your submitted code does not make things worse or
+point you to other patches are already being worked on.
+
+### System properties
+
+In some areas of the code, system properties such as
+* Code size
+* Performance
+* Scalability
+* Latency
+* Complexity
+* &c
+are also important during code reviews.
+
+### Style
+
+* Comments, carriage returns, **snuggly braces**, &c
+* See [CODING_STYLE][5] and [tools/libxl/CODING_STYLE][6]
+* No extraneous whitespace changes
+
+### Documentation and testing
+
+* If there is pre-existing documentation in the tree, such as man pages, design
+ documents, etc. a contributor may be asked to update the documentation
+ alongside the change. Documentation is typically present in the [docs][7]
+ folder.
+* When adding new features that have an impact on the end-user,
+ a contributor should include an update to the [SUPPORT.md][8] file.
+ Typically, more complex features require several patch series before it is
+ ready to be advertised in SUPPORT.md
+* When adding new features, a contributor may be asked to provide tests or
+ ensure that existing tests pass
+
+#### Testing for the Xen Project Hypervisor
+
+Tests are typically located in one of the following directories
+* **Unit tests**: [tools/tests][9] or [xen/test][A]<br>
+ Unit testing is hard for a system like Xen and typically requires building a
+ subsystem of your tree. If your change can be easily unit tested, you should
+ consider submitting tests with your patch.
+* **Build and smoke test**: see [Xen GitLab CI][B]<br>
+ Runs build tests for a combination of various distros and compilers against
+ changes committed to staging. Developers can join as members and test their
+ development branches **before** submitting a patch.
+* **XTF tests** (microkernel-based tests): see [XTF][C]<br>
+ XTF has been designed to test interactions between your software and hardware.
+ It is a very useful tool for testing low level functionality and is executed
+ as part of the project's CI system. XTF can be easily executed locally on
+ xen.git trees.
+* **osstest**: see [README][D]<br>
+ Osstest is the Xen Projects automated test system, which tests basic Xen use
+ cases on a variety of different hardware. Before changes are committed, but
+ **after** they have been reviewed. A contributor’s changes **cannot be
+ applied to master** unless the tests pass this test suite. Note that XTF and
+ other tests are also executed as part of osstest.
+
+### Patch / Patch series information
+
+* Informative one-line changelog
+* Full changelog
+* Motivation described
+* All important technical changes mentioned
+* Changes since previous revision listed
+* Reviewed-by’s and Acked-by’s dropped if appropriate
+
+More information related to these items can be found in our
+[Patch submission Guide][E].
+
+## Code Review Workflow
+
+This section is important for code authors and reviewers. We recommend that in
+particular new code authors carefully read this section.
+
+### Workflow from a Reviewer's Perspective
+
+Patch series typically contain multiple changes to the codebase, some
+transforming the same section of the codebase multiple times. It is quite common
+for patches in a patch series to rely on the previous ones. This means that code
+reviewers review patches and patch series **sequentially** and **the structure
+of a patch series guides the code review process**. Sometimes in a long series,
+patches {1,2}/10 will be clean-ups, {3-6}/10 will be general reorganizations
+which don't really seem to do anything and then {7-10}/10 will be the substance
+of the series, which helps the code reviewer understand what {3-6}/10 were
+about.
+
+Generally there are no hard rules on how to structure a series, as the structure
+of a series is very code specific and it is hard to give specific advice. There
+are some general tips which help and some general patterns.
+
+**Tips:**
+
+* Outline the thinking behind the structure of the patch series. This can make
+ a huge difference and helps ensure that the code reviewer understands what the
+ series is trying to achieve and which portions are addressing which problems.
+* Try and keep changes that belong to a subsystem together
+* Expect that the structure of a patch series sometimes may need to change
+ between different versions of a patch series
+* **Most importantly**: Start small. Don't submit a large and complex patch
+ series as the first interaction with the community. Try and pick a smaller
+ task first (e.g. a bug-fix, a clean-up task, etc.) such that you don't have
+ to learn the tools, code and deal with a large patch series all together for
+ the first time.
+
+**General Patterns:**
+
+If there are multiple subsystems involved in your series, then these are best
+separated out into **sets of patches**, which roughly follow the following
+seven categories. In other words: you would end up with **7 categories x N
+subsystems**. In some cases, there is a **global set of patches** that affect
+all subsystems (e.g. headers, macros, documentation) impacting all changed
+subsystems which ideally comes **before** subsystem specific changes.
+
+The seven categories typically making up a logical set of patches
+1. Cleanups and/or new Independent Helper Functions
+2. Reorganizations
+3. Headers, APIs, Documentation and anything which helps understand the
+ substance of a series
+4. The substance of the change
+5. Cleanups of any infelicities introduced temporarily
+6. Deleting old code
+7. Test code
+
+Note that in many cases, some of the listed categories are not always present
+in each set, as they are not needed. Of course, sometimes there are several
+patches describing **changes of substance**, which could be ordered in different
+ways: in such cases it may be necessary to put reorganizations in between these
+patches.
+
+If a series is structured this way, it is often possible to agree early on,
+that a significant portion of the changes are fine and to check these in
+independently of the rest of the patch series. This means that there is
+* Less work for authors to rebase
+* Less cognitive overhead for reviewers to review successive versions of a
+ series
+* The possibility for different code reviewers to review portions of such
+ large changes independently
+
+**Trade-Offs:**
+
+* In some cases, following the general pattern above may create extra patches
+ and may make a series more complex and harder to understand.
+* Crafting a more extensive cover letter will be extra effort: in most cases,
+ the extra time investment will be saving time during the code review process.
+ Verbosity is not the goal, but clarity is. Before you send a larger series
+ in particular: try and put yourself into the position of a code reviewer and
+ try to identify information that helps a code reviewer follow the patch
+ series.
+* In cases where changes need to be back-ported to older releases, moving
+ general cleanups last is often preferable: in such cases the **substance of
+ the change** is back-ported, whereas general cleanups and improvements are
+ not.
+
+**Example:**
+* [[PATCH v3 00/18] VM forking][H] is a complex patch series with an exemplary
+ cover letter. Notably, it contains the following elements
+ * It provides a description of the design goals and detailed description
+ of the steps required to fork a VM.
+ * A description of changes to the user interface
+ * It contains some information about the test status of the series including
+ some performance information.
+ * It maps the series onto the categories listed above. As expected, not
+ all categories are used in this case. However, the series does contain
+ elements of **1** (in this case preparation to enable the functionality),
+ **2** reorganizations and other non-functional changes that enable the
+ rest of the series and **4** the substance of the series with additional
+ information to make it easier for the reviewer to parse the series.
+
+### Workflow from an Author's Perspective
+
+When code authors receive feedback on their patches, they typically first try
+to clarify feedback they do not understand. For smaller patches or patch series
+it makes sense to wait until receiving feedback on the entire series before
+sending out a new version addressing the changes. For larger series, it may
+make sense to send out a new revision earlier.
+
+As a reviewer, you need some system that helps ensure that you address all
+review comments. This can be tedious when trying to map a hierarchical e-mail
+thread onto a code-base. Different people use different techniques from using
+* In-code TODO statements with comment snippets copied into the code
+* To keeping a separate TODO list
+* To printing out the review conversation tree and ticking off what has been
+ addressed
+* A combination of the above
+
+### <a name="problems"></a>Problematic Patch Reviews
+
+A typical waterfall software development process is sequential with the
+following steps: define requirements, analyze, design, code, test and deploy.
+Problems uncovered by code review or testing at such a late stage can cause
+costly redesign and delays. The principle of **[Shift Left][D]** is to take a
+task that is traditionally performed at a late stage in the process and perform
+that task at earlier stages. The goal is to save time by avoiding refactoring.
+
+Typically, problematic patch reviews uncover issues such as wrong or missed
+assumptions, a problematic architecture or design, or other bugs that require
+significant re-implementation of a patch series to fix the issue.
+
+The principle of **Shift Left** also applies in code reviews. Let's assume a
+series has a major flaw: ideally, this flaw would be picked up in the **first
+or second iteration** of the code review. As significant parts of the code may
+have to be re-written, it does not make sense for reviewers to highlight minor
+issues (such as style issues) until major flaws have been addressed of the
+affected part of a patch series. In such cases, providing feedback on minor
+issues reviewers cause the code author and themselves extra work by asking for
+changes to code, which ultimately may be changed later.
+
+To make it possible for code reviewers to identify major issues early, it is
+important for code-authors to highlight possible issues in a cover letter and
+to structure a patch series in such a way that makes it easy for reviewers to
+separate difficult and easy portions of a patch series. This will enable
+reviewers to progress uncontroversial portions of a patch independently from
+controversial ones.
+
+### Reviewing for Patch Authors
+
+The following presentation by George Dunlap, provides an excellent overview on
+how we do code reviews, specifically targeting non-maintainers.
+
+As a community, we would love to have more help reviewing, including from **new
+community members**. But many people
+* do not know where to start, or
+* believe that their review would not contribute much, or
+* may feel intimidated reviewing the code of more established community members
+
+The presentation demonstrates that you do not need to worry about any of these
+concerns. In addition, reviewing other people's patches helps you
+* write better patches and experience the code review process from the other
+ side
+* and build more influence within the community over time
+
+Thus, we recommend strongly that **patch authors** read the watch the recording
+or read the slides:
+* [Patch Review for Non-Maintainers slides][F]
+* [Patch Review for Non-Maintainers recording - 20"][G]
+
+[1]: communication-practice.md
+[2]: resolving-disagreement.md
+[3]: https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches
+[4]: https://wiki.xenproject.org/wiki/Managing_Xen_Patches_with_Git
+[5]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE
+[6]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/CODING_STYLE
+[7]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=docs
+[8]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=SUPPORT.md
+[9]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=tools/tests
+[A]: https://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=xen/test
+[B]: https://gitlab.com/xen-project/xen/pipelines
+[C]: https://xenbits.xenproject.org/docs/xtf/
+[D]: https://xenbits.xenproject.org/gitweb/?p=osstest.git;a=blob;f=README
+[E]: https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches
+[D]: https://devopedia.org/shift-left
+[F]: https://www.slideshare.net/xen_com_mgr/xpdds19-keynote-patch-review-for-nonmaintainers-george-dunlap-citrix-systems-uk-ltd
+[G]: https://www.youtube.com/watch?v=ehZvBmrLRwg
+[H]: https://lists.xenproject.org/archives/html/xen-devel/2019-12/threads.html#02097
--- /dev/null
+# Communication Guide
+
+We believe that our [Code of Conduct](code-of-conduct.md) can help create a
+harassment-free environment, but is not sufficient to create a welcoming
+environment on its own. We can all make mistakes: when we do, we take
+responsibility for them and try to improve.
+
+This document lays out our gold standard, best practices for some common
+situations and mechanisms to help resolve issues that can have a
+negative effect on our community.
+
+## Goal
+
+We want a productive, welcoming and agile community that can welcome new
+ideas in a complex technical field which is able to reflect on and improve how
+we work.
+
+## Communication & Handling Differences in Opinions
+
+Examples of behavior that contributes to creating a positive environment
+include:
+* Use welcoming and inclusive language
+* Keep discussions technical and actionable
+* Be respectful of differing viewpoints and experiences
+* Be aware of your own and counterpart’s communication style and culture
+* Gracefully accept constructive criticism
+* Focus on what is best for the community
+* Show empathy towards other community members
+* Resolve differences in opinion effectively
+
+## Getting Help
+
+When developing code collaboratively, technical discussion and disagreements
+are unavoidable. Our contributors come from different countries and cultures,
+are driven by different goals and take pride in their work and in their point
+of view. This invariably can lead to lengthy and unproductive debate,
+followed by indecision, sometimes this can impact working relationships
+or lead to other issues that can have a negative effect on our community.
+
+To minimize such issue, we provide a 3-stage process
+* Self-help as outlined in this document
+* Ability to ask for an independent opinion or help in private
+* Mediation between parties which disagree. In this case a neutral community
+ member assists the disputing parties resolve the issues or will work with the
+ parties such that they can improve future interactions.
+
+If you need and independent opinion or help, feel free to contact
+mediation@xenproject.org. The team behind mediation@ is made up of the
+same community members as those listed in the Conduct Team: see
+[Code of Conduct](code-of-conduct.md). In addition, team members are obligated
+to maintain confidentiality with regard discussions that take place. If you
+have concerns about any of the members of the mediation@ alias, you are
+welcome to contact precisely the team member(s) of your choice. In this case,
+please make certain that you highlight the nature of a request by making sure
+that either help or mediation is mentioned in the e-mail subject or body.
+
+## Specific Topics and Best Practice
+
+* [Code Review Guide](code-review-guide.md):
+ Essential reading for code reviewers and contributors
+* [Communication Best Practice](communication-practice.md):
+ This guide covers communication guidelines for code reviewers and authors.
+ It should help you create self-awareness, anticipate, avoid and help resolve
+ communication issues.
+* [Resolving Disagreement](resolving-disagreement.md):
+ This guide lays out common situations that can lead to dead-lock and shows
+ common patterns on how to avoid and resolve issues.
--- /dev/null
+# Communication Best Practice
+
+This guide provides communication Best Practice that helps you in
+* Using welcoming and inclusive language
+* Keeping discussions technical and actionable
+* Being respectful of differing viewpoints and experiences
+* Being aware of your own and counterpart’s communication style and culture
+* Show empathy towards other community members
+
+## Code reviews for **reviewers** and **patch authors**
+
+Before embarking on a code review, it is important to remember that
+* A poorly executed code review can hurt the contributors feeling, even when a
+ reviewer did not intend to do so. Feeling defensive is a normal reaction to
+ a critique or feedback. A reviewer should be aware of how the pitch, tone,
+ or sentiment of their comments could be interpreted by the contributor. The
+ same applies to responses of an author to the reviewer.
+* When reviewing someone's code, you are ultimately looking for issues. A good
+ code reviewer is able to mentally separate finding issues from articulating
+ code review comments in a constructive and positive manner: depending on your
+ personality this can be **difficult** and you may need to develop a technique
+ that works for you.
+* As software engineers we like to be proud of the solutions we came up with.
+ This can make it easy to take another people’s criticism personally. Always
+ remember that it is the code that is being reviewed, not you as a person.
+* When you receive code review feedback, please be aware that we have reviewers
+ from different backgrounds, communication styles and cultures. Although we
+ are all trying to create a productive, welcoming and agile environment, we do
+ not always succeed.
+
+### Express appreciation
+
+As the nature of code review to find bugs and possible issues, it is very easy
+for reviewers to get into a mode of operation where the patch review ends up
+being a list of issues, not mentioning what is right and well done. This can
+lead to the code submitter interpreting your feedback in a negative way.
+
+The opening of a code review provides an opportunity to address this and also
+sets the tone for the rest of the code review. Starting **every** review on a
+positive note, helps set the tone for the rest of the review.
+
+For an initial patch, you can use phrases such as
+> Thanks for the patch
+> Thanks for doing this
+
+For further revisions within a review, phrases such as
+> Thank you for addressing the last set of changes
+
+If you believe the code was good, it is good practice to highlight this by
+using phrases such as
+> Looks good, just a few comments
+> The changes you have made since the last version look good
+
+If you think there were issues too many with the code to use one of the
+phrases, you can still start on a positive note, by for example saying
+> I think this is a good change
+> I think this is a good feature proposal
+
+It is also entirely fine to highlight specific changes as good. The best place
+to do this, is at the top of a patch, as addressing code review comments
+typically requires a contributor to go through the list of things to address
+and an in-lined positive comment is likely to break that workflow.
+
+You should also consider, that if you review a patch of an experienced
+contributor phrases such as *Thanks for the patch* could come across as
+patronizing, while using *Thanks for doing this* is less likely to be
+interpreted as such.
+
+Appreciation should also be expressed by patch authors when asking for
+clarifications to a review or responding to questions. A simple
+> Thank you for your feedback
+> Thank you for your reply
+> Thank you XXX!
+
+is normally sufficient.
+
+### Avoid inflammatory and negatively charged language
+
+The way a reviewer expresses feedback, has a big impact on how the author
+perceives the feedback. Choosing negatively charged language such as your code
+is terrible, fragile, spaghetti, inefficient, racy, messy, etc. creates a
+negative emotional response in the submitter, which can then make subsequent
+communication difficult. The same is true when a patch author is responding to
+a comment from a reviewer.
+
+One of our maintainers has been studying Mandarin for several years and has
+come across the most strongly-worded dictionary entry [he has ever seen][1].
+This example illustrates the differences between an inflammatory and fact-based
+description extremely well.
+
+> 裹脚 (guo3 jiao3): foot-binding (a vile feudal practice which crippled women
+> both physically and spiritually)
+
+This is not something one is used to hearing from dictionary entries. Once you
+investigate the practice foot-binding, it is hard to disagree with the
+dictionary entry. However, the statement does not contain much information. If
+you read it without knowing what foot-binding is, it is hard to be convinced
+by this statement. The main take-away is that the author of the dictionary
+entry had strong opinions about this topic. It does not tell you why you
+should have the same opinion.
+
+Compare this to the [Wikipedia entry][2]
+
+> Foot binding was the custom of applying tight binding to the feet of young
+> girls to modify the shape and size of their feet. ... foot binding was a
+> painful practice and significantly limited the mobility of women, resulting
+> in lifelong disabilities for most of its subjects. ... Binding usually
+> started during the winter months since the feet were more likely to be numb,
+> and therefore the pain would not be as extreme. …The toes on each foot
+> were curled under, then pressed with great force downwards and squeezed
+> into the sole of the foot until [redacted] ...
+
+Without going into the details of foot-binding, it is noticeable that the
+definition is a list of simple facts that are laid out in a way that make it
+obvious what the correct conclusion is.
+
+Because the Wikipedia entry is entirely fact based it is more powerful and
+persuasive than the dictionary entry. The same applies to code reviews.
+
+Making statements in code reviews such as
+> Your code is garbage
+> This idea is stupid
+
+besides negatively charged, rude and counter productive
+* It will make the patch author angry: instead of finding a solution to the
+ problem the author will spend time and mental energy wrestling with their
+ feelings
+* It does not contain any information
+* Facts are both more powerful and more persuasive
+
+Consider the following two pieces of feedback on a piece of code
+> This piece of code is confusing
+> It took me a long time to figure out what was going on here
+
+The first example expresses an opinion, whereas the second re-phrases the
+statement in terms of what you experienced, which is a fact.
+
+Other examples:
+> BAD: This is fragile
+> SOMEWHAT BETTER: This seems fragile to me
+> BEST: If X happens, Y will happen.
+
+A certain piece of code can be written in many different ways: this can lead to
+disagreements on the best architecture, design or coding pattern. As already
+pointed out in this section: avoid feedback that is opinion-based and thus
+does not add any value. Back your criticism (or idea on how to solve a
+problem) with a sensible rationale.
+
+### Review the code, not the person
+
+Without realizing it, it is easy to overlook the difference between insightful
+critique of code and personal criticism. Let's look at a theoretical function
+where there is an opportunity to return out of the function early. In this
+case, you could say
+
+> You should return from this function early, because of XXX
+
+On its own, there is nothing wrong with this statement. However, a code review
+is made up of multiple comments and using **You should** consistently can
+start to feel negative and can be mis-interpreted as a personal attack. Using
+something like avoids this issue:
+
+> Returning from this function early is better, because of XXX
+
+Without personal reference, a code review will communicate the problem, idea
+or issue without risking mis-interpretation.
+
+### Verbose vs. terse
+
+Due to the time it takes to review and compose code reviewer, reviewers often
+adopt a terse style. It is not unusual to see review comments such as
+> typo
+> s/resions/regions/
+> coding style
+> coding style: brackets not needed
+etc.
+
+Terse code review style has its place and can be productive for both the
+reviewer and the author. However, overuse can come across as unfriendly,
+lacking empathy and can thus create a negative impression with the author of a
+patch. This is in particular true, when you do not know the author or the
+author is a newcomer. Terse communication styles can also be perceived as rude
+in some cultures.
+
+If you tend to use a terse commenting style and you do not know whether the
+author is OK with it, it is often a good idea to compensate for it in the code
+review opening (where you express appreciation) or when there is a need for
+verbose expression. However, when you know are working with a seasoned code
+author, it is also entirely acceptable to drop niceties such as expressing
+appreciation with the goal to save the author and reviewer time.
+
+It is also entirely fine to mention that you have a fairly terse communication
+style and ask whether the author is OK with it. In almost all cases, they will
+be: by asking you are showing empathy that helps counteract a negative
+impression.
+
+### Clarity over verbosity
+
+When reading this document, you may get the impression that following the
+guidance outlined here takes more effort and time for both code reviewers and
+code authors. This is not the intention: much of this document aims to create
+clearer communication, which ultimately saves time by reducing unnecessary
+iterations during communication. We value **clarity over verbosity**.
+
+Areas which often create unnecessary back-and-forth between reviewers and
+authors are
+* Unstated assumptions and goals
+* Leave suggestions, examples, and resources (such as links to existing code)
+* There is nothing more helpful for the thought process than example. It
+ guarantees that you have a shared understanding and reduces the questions
+ asked on a comment.
+
+### Code Review Comments should be actionable
+
+Code review comments should be actionable: in other words, it needs to be clear
+what the author of the code needs to do to address the issue you identified.
+
+Statements such as
+> BAD: This is wrong
+> BAD: This does not work
+> BETTER, BUT NOT GOOD: This does not work, because of XXX
+
+do not normally provide the author of a patch with enough information to send
+out a new patch version. By doing this, you essentially force the patch author
+to **find** and **implement** an alternative, which then may also not be
+acceptable to you as the **reviewer** of the patch.
+
+A better way to approach this is to say
+
+> This does not work, because of XXX
+> You may want to investigate YYY and ZZZ as alternatives
+
+In some cases, it may not be clear whether YYY or ZZZ are the better solution.
+As a reviewer you should be as up-front and possible in such a case and say
+something like
+
+> I am not sure whether YYY and ZZZ are better, so you may want to outline your
+> thoughts about both solutions by e-mail first, such that we can decide what
+> works best
+
+### Identify the severity and optionality of review comments
+
+By default, every comment which is made **ought to be addressed** by the
+author. However, sometimes reviewers note issues, which would be nice if they
+were addressed, but are not mandatory to fix.
+
+Typically, reviewers use terminology such as
+> This would be a nice-to-have
+> This is not a blocker
+
+Some maintainers use
+> NIT: XXX
+
+however, it is sometimes also used to indicate a minor issue that **must** be
+fixed. Also terminology such as **this is not a blocker** could be
+misinterpreted. It is important that **reviewers** use language that make
+clear whether a comment is an optional suggestion. Examples may be
+> NIT (optional): XXX
+> I think it would be good if X also did Y, not a requirement but nice-to-have
+
+### Identify the severity of a disagreement
+
+During a code review, it can happen that reviewer and author disagree on how
+to move forward. The default position when it comes to disagreements is that
+**both parties want to argue their case**. However, frequently one or both
+parties do not feel that strongly about a specific issue.
+
+Within the Xen Project, we have [a way][3] to highlight one's position on
+proposals, formal or informal votes using the following notation:
+> +2 : I am happy with this proposal, and I will argue for it
+> +1 : I am happy with this proposal, but will not argue for it
+> 0 : I have no opinion
+> -1 : I am not happy with this proposal, but will not argue against it
+> -2 : I am not happy with this proposal, and I will argue against it
+
+You can use a phrase such as
+> I am not happy with this suggestion, but will not argue against it
+
+to make clear where you stand, while recording your position. Conversely, a
+reviewer may do something similar
+> I am not happy with XYZ, but will not argue against it [anymore]
+> What we have now is good enough, but could be better
+
+### Authors: responding to review comments
+
+Typically patch authors are expected to **address all** review comments in the
+next version of a patch or patch series. In a smooth-running code review where
+you do not have further questions it is not at all necessary to acknowledge
+the changes you are going to make:
+* Simply send the next version with the changes addressed and record it in the
+ change-log
+
+When there is discussion, the normal practice is to remove the portion of the
+e-mail thread where there is agreement. Otherwise, the thread can become
+exceptionally long.
+
+In cases where there was discussion and maybe disagreement, it does however
+make sense to close the discussion by saying something like
+
+> ACK
+> Seems we are agreed, I am going to do this
+
+Other situations when you may want to do this are cases where the reviewer made
+optional suggestions, to make clear whether the suggestion will be followed or
+not.
+
+### Avoid uncommon words: not everyone is a native English speaker
+
+Avoid uncommon words both when reviewing code or responding to a review. Not
+everyone is a native English speaker. The use of such words can come across
+badly and can lead to misunderstandings.
+
+### Prioritize significant flaws
+
+If a patch or patch series has significant flaws, such as
+* It is built on wrong assumptions
+* There are issues with the architecture or the design
+
+it does not make sense to do a detailed code review. In such cases, it is best
+to focus on the major issues first and deal with style and minor issues in a
+subsequent review. Not all series have significant flaws, but most series have
+different classes of changes that are required for acceptance: covering a
+range of major code modifications to minor code style fixes. To avoid
+misunderstandings between reviewers and contributors, it is important to
+establish and agree whether a series or part of a series has a significant
+flaw and agree a course of action.
+
+A pragmatic approach would be to
+* Highlight problematic portions of a series in the cover letter
+* For the patch author and reviewer(s) to agree that for problematic to omit
+ style and minor issues in the review, until the significant flaw is addressed
+
+This saves both the patch author and reviewer(s) time. Note that some
+background is covered in detail in [Problematic Patch Reviews][4].
+
+
+### Reviewers: Welcome newcomers
+
+When reviewing the first few patches of a newcomer to the project, you may want
+spend additional time and effort in your code review. This contributes to a
+more **positive experience**, which ultimately helps create a positive working
+relationship in the long term.
+
+When someone does their first code submission, they will not be familiar with
+**all** conventions in the project. A good approach is to
+* Welcome the newcomer
+* Offer to help with specific questions, for example on IRC
+* Point to existing documentation: in particular if mistakes with the
+ submission itself were made. In most situations, following the submission
+ process makes the process more seamless for the contributor. So, you could
+ say something like
+
+> Hi XXX. Welcome to the community and thank you for the patch
+>
+> I noticed that the submission you made seems to not follow our process.
+> Are you aware of this document at YYY? If you follow the instructions the
+> entire code submission process and dealing with review comments becomes
+> much easier. Feel free to find me on IRC if you need specific help. My IRC
+> handle is ZZZ
+
+### Reviewers: Take account of previous reviewer(s) comments
+
+Sometimes multiple reviewers share reviewing a series. For example,
+reviewer John has reviewed the first 5 iterations of the series. The patch
+author has addressed all of John's comments and Susan comes in and picks up
+the series after iteration 5. In such cases it is possible that John and Susan
+have different styles, such as
+* different preferences on the code layout
+* different preferences on code style
+
+If Susan were to be strict on her own style and highlight her style
+preferences in subsequent reviews, this would cause additional re-work for the
+code author. In addition, it also causes extra work for Susan. The easiest way
+to avoid such situations, would be for Susan to focus on faulty code only and
+to disregard personal preferences when taking over the review of a series.
+
+### Reviewers: Review the code, then review the review
+
+As stated earlier it is often difficult to mentally separate finding issues
+from articulating code review comments in a constructive and positive manner.
+Even as an experienced code reviewer you can be in a bad mood, which can
+ impact your communication style.
+
+A good trick to avoid this, is to start and complete the code review and then
+**not send it immediately**. You can then have a final go over the code review
+at some later point in time and review your comments from the other author's
+point of view. This minimizes the risk of being misunderstood. The same
+applies when replying to a code review: draft your reply and give it a final
+scan before pressing the send button.
+
+Generally, it is a good idea for code reviewers to do this regularly, purely
+from the viewpoint of self-improvement and self-awareness.
+
+## Common Communication Pitfalls
+
+This section contains common communication issues and provides suggestions on
+how to avoid them and resolve them. These are **general** issues which affect
+**all** online communication. As such, we can only try and do our best.
+
+### Misunderstandings
+
+When you meet face to face, you can read a person’s emotions. Even with a
+phone call, someone’s tone of voice can convey a lot of information. Using
+on-line communication channels you are flying blind, which often leads to
+misunderstandings. [Research][5] shows that in up to 50% of email
+conversations, the tone of voice is misinterpreted.
+
+In code reviews and technical discussions in general we tend to see two things
+* The reviewer or author interprets an exchange as too critical, passive
+ aggressive, or other: this usually comes down to different cultures and
+ communication styles, which are covered in the next section
+* There is an actual misunderstanding of a subject under discussion
+
+In the latter case, the key to resolution is to **identify the
+misunderstanding** as quickly as possible and call it out and de-escalate
+rather than let the misunderstanding linger. This is inherently difficult and
+requires more care than normal communication. Typically you would start with
+* Showing appreciation
+* Highlighting the potential misunderstanding and verifying whether the other
+ person also feels that maybe there was a misunderstanding
+* Proposing a way forward: for example, it may make sense to move the
+ conversation from the mailing list to [IRC][6] either in private or public,
+ a community call or a private phone/video call.
+
+It is entirely acceptable to do this in a direct reply to your communication
+partner, rather than on a public e-mail list on or an otherwise public forum.
+
+A good approach is to use something like the following:
+> Hi XXX! Thank you for the insights you have given me in this code review
+> I feel that we are misunderstanding each other on the topic of YYY
+> Would you mind trying to resolve this on IRC. I am available at ZZZ
+
+Usually, technical misunderstandings come down two either
+1. Misinterpreting what the other person meant
+2. Different - usually unstated - assumptions on how something works or what
+ is to be achieved
+3. Different - usually unstated - objectives and goals, which may be
+ conflicting
+4. Real differences in opinion
+
+The goal of calling out a possible misunderstanding is to establish what
+caused the misunderstanding, such that all parties can move forward.
+Typically, 1 and 2 are easily resolved and will lead back to a constructive
+discussion. Whereas 3 and 4 may highlight an inherent disagreement, which may
+need to be resolved through techniques as
+outlined in [Resolving Disagreement][7].
+
+### Cultural differences and different communication styles
+
+The Xen Project is a global community with contributors from many different
+backgrounds. Typically, when we communicate with a person we know, we factor
+in past interactions. The less we know a person, the more we rely on cultural
+norms.
+
+However, different norms and value systems come into play when people from
+diverse cultural backgrounds interact. That can lead to misunderstandings,
+especially in sensitive situations such as conflict resolution, giving and
+receiving feedback, and consensus building.
+
+For example, giving direct feedback such as
+> [Please] replace XXX with YYY, as XXX does not do ZZZ
+
+is acceptable and normal in some cultures, whereas in cultures which value
+indirect feedback it would be considered rude. In the latter case, something
+like the following would be used
+> This looks very good to me, but I believe you should use YYY here,
+> because XXX would....
+
+The key to working and communicating well with people from different cultural
+backgrounds is **self-awareness**, which can then be used to either
+* Adapt your own communication style depending on who you talk to
+* Or to find a middle-ground that covers most bases
+
+A number of different theories in the field of working effectively are
+currently popular, with the most well-known one being
+[Erin Meyer's Culture Map][8]. A short overview can be found [here][9]
+(33 slides).
+
+### Code reviews and discussions are not competitions
+
+Code reviews on our mailing lists are not competitions on who can come up with
+the smartest solution or who is the real coding genius.
+
+In a code review - as well as in general - we expect that all stake-holders
+* Gracefully accept constructive criticism
+* Focus on what is best for the community
+* Resolve differences in opinion effectively
+
+The next section provides pointers on how to do this effectively.
+
+### Resolving Disagreement Effectively
+
+Common scenarios are covered our guide on [Resolving Disagreement][7], which
+lays out situations that can lead to dead-lock and shows common patterns on
+how to avoid and resolve issues.
+
+[1]: https://youtu.be/ehZvBmrLRwg?t=834
+[2]: https://en.wikipedia.org/wiki/Foot_binding
+[3]: https://xenproject.org/developers/governance/#expressingopinion
+[4]: resolving-disagreement.md#problems
+[5]: https://www.wired.com/2006/02/the-secret-cause-of-flame-wars/
+[6]: https://xenproject.org/help/irc/
+[7]: resolving-disagreement.md
+[8]: https://en.wikipedia.org/wiki/Erin_Meyer
+[9]: https://www.nsf.gov/attachments/134059/public/15LFW_WorkingWithMulticulturalTeams_LarsonC.pdf
:caption: Contents:
-
Indices and tables
==================
--- /dev/null
+# 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 fulfill 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 installment, 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 code 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@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 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
projects / governance.git / commitdiff