1# Reviewer 2 3A review is given by a reviewer on code contributions (see: [contributing.md](contributing.md)) of 4a reviewee. There are no guidelines on how review has to be performed in ILIAS, however here we propose a pattern 5inspired by an article of Dan Munckton in [How to be a kinder more effective code reviewer](https://cultivatehq.com/posts/how-to-be-a-kinder-more-effective-code-reviewer/). 6 7In our experience many conflicts arise due to feedback written in a fashion which makes it hard to understand, 8what exactly is asked of a reviewee. The pattern adapted from suggestions by Munckton is aimed 9to make reviews easier to read and to leave less room for misunderstandings and communicational deadlocks. 10 11 12 13## General Pattern 14We try to lay a strong focus on making feedback **actionable**, meaning the reviewee should have 15a clear picture on how to react. Inspired by Munckton (see below for more details) we propose 16to use the following structure to reply to PRs: 17 18``` 19Hi @[name_of_reviewee] 20 21Thank you a lot for contributing to ILIAS. 22 23[reaction_not_actionable] 24 25Please answer the following questions: 26- [ ] [question1]? [optional: reasoning behind the question] 27- [ ] [question2]? [optional: reasoning behind the question] 28- ... 29 30Please consider the following suggestions. You do not need to follow those, but but please indicate shortly why you prefer to do otherwise: 31- [ ] [suggestion1] [optional: reasoning behind the suggestion] 32- [ ] [suggestion2] [optional: reasoning behind the suggestion] 33- ... 34 35Please implement the following changes: 36- [ ] [change_request_1] [optional: reasoning behind the suggestion] 37- [ ] [change_request_2] [optional: reasoning behind the suggestion] 38- ... 39 40[optional_comments_on_how_to_proceed] 41 42kindly, 43@[your_username] 44 45``` 46This might look as follows: 47 48``` 49Hi @klees 50 51Thank you a lot for contributing to ILIAS. 52 53We highly apreciate the effort you took to improve the JS in the KS component. I especially 54like the new proposed pattern to tackle the scoping issue. 55 56Please answer the following questions: 57- [ ] Why did you not use ECMAScript 6? Note that Bootstrap 4 makes use of it. 58 59Please consider the following suggestions. You do not need to follow those, but but please indicate shortly 60why you prefer to do otherwise: 61- [ ] Change XY to pattern XZ. We believe that pattern XZ might be superior. However, I am not completely sure. 62 63 64Please implement the following changes: 65- [ ] Reword the function addProperty to withProperty. withProperty is in line with our naming scheme used 66 in othe methods of the framework. 67 68 69Please give a feedback until the end of the week indicating how long it will take to answer 70the questions given, react to the suggestions and implement our change requests. 71 72 73kindly, 74@amstutz 75 76``` 77 78In short, we have the following suggestions for you when writting reviews: 79- [ ] Use the above template. Reasoning: See next chapter. 80 81## Reasoning 82Munckton introduces the term **actionable** for feedbacks. An **actionable** feedback is one that 83can directly be reacted upon. He claims Questions, Suggestions and Change Requests 84to be the most relevant types of such statements. He gives the following examples of those three statements: 85 86- Question: could you explain why this ended up having to be here? 87- Suggestion: you may have come across this already, but we might be able to use XYZ for this. 88- Change request: sorry, I appreciate the effort it took to get it this far. But the team 89already agreed to solve this using XYZ. So I'm going to have to ask you to rework this. Let me know if I can help. 90 91 92A fourth type he sometimes uses is the reaction, which is not actionable, but might help to express emotions such as thanks. 93 94He further suggests to always keep things as clear as possible by using the following pattern: 95 96>[type]: [actionable request] 97 98>[rationale or discussion] 99 100Whereas: 101- type: one of Question, Suggestion or Changes Request. 102- actionable request: is a short clear question or statement, which makes clear how the reviewee 103should respond. 104- rational or discussion: Possible further explanations of the reasoning behind the request. 105 106An example could be: 107>Suggestion: use XYZ. 108 109>You may have come across this already, but we might be able to use XYZ for this because blah blah blah etc. 110 111We adapted this slightly by bundling questions, suggestions and change requests in grouped listings and placing 112the reactions up front. 113 114## Inline Comments 115Github makes it very easy to leave inline comments. However, in the past, we observed, that large 116numbers of inline comments make a review hard to read and understand. We therefore propose 117only the leave inline comments for trivial directly actionable suggestions and change request, that 118do not need an rational (E.g. *Change Request: typo in word actionalbe*). 119 120Do not leave any reactions in inline comments. They tend to crowd the discussion without giving 121the possibility of being resolved (seens not actionable). Move them to the summary. 122 123 124 125 126