|
| 1 | +Giving constructive criticism |
| 2 | +============================= |
| 3 | + |
| 4 | +:doc:`Reviewing issues and pull request </contributing/community/reviews>` is a great way to get started with |
| 5 | +contributing to the Symfony community. Anyone can do it! But before |
| 6 | +you give a comment, take a step back and think, is what you are about |
| 7 | +to say actually what you indent? |
| 8 | + |
| 9 | +Communicating over the internet with nothing but text can pose a |
| 10 | +big challenge, especially if you remember the Symfony community |
| 11 | +is world-wide and is composed of a wide variety of people with differing |
| 12 | +idea's and opinions. |
| 13 | + |
| 14 | +Not everyone speaks English, is able to use a keyboard, and some might |
| 15 | +even have dyslexia. And not to mention that some might have a |
| 16 | +bad experience from previous contributions (to other projects). |
| 17 | + |
| 18 | +Your not alone in this, this guide will try help you write |
| 19 | +constructive, respectful and helpful reviews and replies. |
| 20 | + |
| 21 | +.. tip:: |
| 22 | + |
| 23 | + This guide is not about lecturing you to "conform" or give-up |
| 24 | + your ideas and opinions. But helping you to better communicate, |
| 25 | + prevent possible confusion, and keeping the Symfony community a |
| 26 | + welcoming place for everyone. **You are free to disagree with |
| 27 | + someone's opinions just don't be disrespectful.** |
| 28 | + |
| 29 | +First of, accept that many programming decisions are opinions. |
| 30 | +Discuss trade offs, which you prefer, and reach a resolution quickly. |
| 31 | +It's not about being right or wrong, but using what works. |
| 32 | + |
| 33 | +Tone of voice |
| 34 | +------------- |
| 35 | + |
| 36 | +We don't expect you to be completely formal, or to even write error-free |
| 37 | +English. Just remember one thing: Don't swear, and be respectful to others |
| 38 | + |
| 39 | +Don't reply in anger or with an aggressive tone. You're angry, we understand |
| 40 | +that, but swearing/cursing and name calling doesn't really encourage anyone to |
| 41 | +help you. Take a deep breath, count to 10 and try to *clearly* explain what problems |
| 42 | +you encounter. |
| 43 | + |
| 44 | +Gender pronouns |
| 45 | +--------------- |
| 46 | + |
| 47 | +While not "formally" required it's better to use gender-neutral pronouns. |
| 48 | +Unless someone "indicated" there pronouns, use "they", "them" instead of |
| 49 | +"he", "she", "his", "hers", "his/hers", "he/she", etc. |
| 50 | + |
| 51 | +And instead of saying "guys" it's better to use "folks", or Symfonions |
| 52 | +if you like that. |
| 53 | + |
| 54 | +Giving positive feedback |
| 55 | +------------------------ |
| 56 | + |
| 57 | +While reviewing issues and pull request you may run into some suggestions |
| 58 | +(including patches) that don't reflect your idea's, are not good, or down |
| 59 | +right wrong. |
| 60 | + |
| 61 | +Now, instead of writing a lengthy comment why you think they are wrong. |
| 62 | +Consider the amount of work and time the author has spend on there idea. |
| 63 | +And how your response would make them feel; |
| 64 | + |
| 65 | +* Are you making a point only because you believe they are wrong? |
| 66 | + |
| 67 | +* Did you correctly understand there intention? or are you making assumptions? |
| 68 | + |
| 69 | +Maybe you can ask for clarification?, suggest an alternative? or |
| 70 | +provide a simple explanation *why* disagree with there proposal. |
| 71 | + |
| 72 | +Even if something *is* really wrong or "a bad idea", stay respectful and |
| 73 | +don't get into endless you-are-wrong discussions or "flame wars". |
| 74 | + |
| 75 | +* Good questions avoid judgment and avoid assumptions about the author's |
| 76 | + perspective. |
| 77 | + |
| 78 | +* Avoid using terms that could be seen as referring to personal traits. ("dumb", |
| 79 | + "stupid"). Assume everyone is intelligent and well-meaning. |
| 80 | + |
| 81 | +* Be explicit. Remember people don't always understand your intentions online. |
| 82 | + |
| 83 | +* Don't use hyperbole. ("always", "never", "endlessly", "nothing", "worst", |
| 84 | + "horrible", "terrible"). |
| 85 | + |
| 86 | +Instead of saying "This is a horrible idea" use something like |
| 87 | +"This can introduce issues in the future. Please make sure this is safe to use." |
| 88 | + |
| 89 | +Asking for changes |
| 90 | +------------------ |
| 91 | + |
| 92 | +Rarely something is perfect from the start, while the code itself is good. |
| 93 | +It may not be optimal or conform the Symfony coding style. |
| 94 | + |
| 95 | +Again, understand the author already spend time on the issue and asking |
| 96 | +for (small) changes may be misinterpreted or seen as a personal attack. |
| 97 | + |
| 98 | +Be thankful for there work (so far), stay positive and really help them |
| 99 | +to make the contribution a great one. *Especially if they are a first |
| 100 | +time contributor.* |
| 101 | + |
| 102 | +Use words like "Please", "Thank you" and "Could you" instead of making demands; |
| 103 | + |
| 104 | +* "Thank you for your work so far. I left some suggestions for improvement |
| 105 | + to make the code more readable." |
| 106 | + |
| 107 | +* "Your code contains some coding-style problems, can you fix these before |
| 108 | + we merge? Thank you" |
| 109 | + |
| 110 | +* "Please use 4 spaces instead of tabs", "This needs be on the previous line"; |
| 111 | + |
| 112 | + During a pull request review you can usually leave more then one comment, |
| 113 | + you don't have to use "Please" all the time. But it wouldn't hurt. |
| 114 | + |
| 115 | +It may not seem like much, but saying "Thank you" does make others feel |
| 116 | +more welcome. |
| 117 | + |
| 118 | +Making jokes (using humor) |
| 119 | +-------------------------- |
| 120 | + |
| 121 | +In short: Don't be a troll; This violates the Code of Conduct and may |
| 122 | +even get you banned! |
| 123 | + |
| 124 | +OK, lets be honest. We all make jokes, some are funny and some are not. |
| 125 | +They don't put anyone of, right? Well, who's to tell. What's funny to one |
| 126 | +is offensive to another. |
| 127 | + |
| 128 | +In short. Keep it real. There is nothing wrong with using a meme or emoji |
| 129 | +from time to time, but don't over do it. And even then, keep it friendly. |
| 130 | + |
| 131 | +**Don't use sarcasm for a serious topic, that's not something that belongs |
| 132 | +to the Symfony community.** And don't marginalize someone's problems; |
| 133 | +"Well I guess that's not suppose to happen? 😆 ". |
| 134 | + |
| 135 | +Even if someone's explanation is "inviting to joke about", it's a real |
| 136 | +problem to them. Making jokes about this doesn't help with solving |
| 137 | +there problem and only makes them *feel stupid*. Instead try to discover |
| 138 | +what the problem is really about. |
| 139 | + |
| 140 | +If emoji, animated gifs, or humor aren't you, don't force them. |
| 141 | + |
| 142 | +Final words |
| 143 | +----------- |
| 144 | + |
| 145 | +Don't feel bad if you "failed" to follow these tips. As long as your |
| 146 | +intentions were good and you didn't really offended or insulted anyone; |
| 147 | +you can explain you misunderstood, you didn't meant to marginalize or |
| 148 | +simply failed. |
| 149 | + |
| 150 | +But don't say it "just because", if your apology is not really meant |
| 151 | +you *will* loose credibility and respect from other developers. |
| 152 | + |
| 153 | +*Don't do unto others what you don't want others to do unto you.* |
| 154 | + |
0 commit comments