The latest articles on DEV Community by kubicle (@kubicle). https://dev.to/kubicle https://media2.dev.to/dynamic/image/width=90,height=90,fit=cover,gravity=auto,format=auto/https:%2F%2Fdev-to-uploads.s3.amazonaws.com%2Fuploads%2Fuser%2Fprofile_image%2F196603%2F087dbb0a-481a-4a2a-a196-ae710f6f0cdd.jpeg https://dev.to/kubicle en kubicle Tue, 13 Aug 2019 10:58:41 +0000 https://dev.to/kubicle/what-i-look-for-in-pr-reviews-3h54 https://dev.to/kubicle/what-i-look-for-in-pr-reviews-3h54 <p>This has been (and will be again) written and explained in length by talented writers, but I thought having a very short list can help.<br> For example you can modify this list as you prefer and share it with junior devs you are about to review, so they can "get there" faster.</p> <h1> The short list </h1> <p>The 4 aspects below are on top of my list for any review:</p> <ul> <li>small functions doing a single task</li> <li>no "copy-paste" code (DRY)</li> <li>good naming of functions and variables</li> <li>separation of concerns</li> </ul> <h1> Some Details </h1> <p>The explanations below are more my own "gut feeling"; people can discuss or disagree endlessly about some of these points. Feel free to adapt/modify according to your own opinion.</p> <h2> Good naming of variables </h2> <p>No need for a comment to understand what a variable contains - or at least easy to remember when you read its comment once.</p> <h2> Good naming of functions </h2> <p>No comment should be needed next to a function call to understand what calling this function does.<br> Comments in the function's code can explain HOW it is done, or WHY it is done this way, of course!</p> <h2> Separation of concerns </h2> <p>Code is where it should be, data and functions are private as much as possible, etc.<br> Note the "etc.": there is a lot to say/explain about how this can be done, and usually too much for a code review.<br> Also, this is often about system design, hence the code review might happen too late to change things.<br> Finally, it is the hardest concept to explain to junior developers.<br> For these reasons, this point is last in my short list. This is a bit like a "Trojan horse" bullet point, with an army of concepts hidden within.</p> github pr codequality