+++ /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
+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 :doc:`Resolving Disagreement <resolving-disagreement>`.
+
+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 :doc:`Resolving Disagreement <resolving-disagreement>`, 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/
+.. _8: https://en.wikipedia.org/wiki/Erin_Meyer
+.. _9: https://www.nsf.gov/attachments/134059/public/15LFW_WorkingWithMulticulturalTeams_LarsonC.pdf
projects / people / gdunlap / governance.git / commitdiff