Methods for writing a code review












3















I currently spend a good deal of time over at Code Review and I would love to improve the quality of the reviews I write. Can you give me any insight into the structure or approach you use when writing a Code Review (be it on Stack Exchange or otherwise.) While I do understand that professional code reviews aren't going to look exactly the same as those on Stack Exchange, I would appreciate a perspective into how you would structure one professionally.



Where do you start? Architecture and design? Algorithm? Seriousness of flaw?



Do you structure it in any meaningful way?



Do you weigh in on controversial "best practices" topics that may not be overtly wrong in this use case?



Do you provide links to articles or Stack Overflow Questions and Answers to support your review?



How far do you go to solve the problem?



What have I missed?










share|improve this question























  • It may help if you link to a review that you have made so we can provide specific tips on things you can improve.

    – linksassin
    2 hours ago











  • @linksassin That strikes me as a critique request so I hesitate to do that. If interested though just look at my answers on CR.SE in my profile.

    – bruglesco
    2 hours ago











  • Fair enough, that's a decent reason not too. An example of the type of Code Review you are talking about might be good though. For instance code reviews conducted in git merge tools look very different to pure text formal reviews. Or are you mainly just focused on the Code Review stack answers?

    – linksassin
    2 hours ago











  • @linksassin I'm not really sure. I don't know what it looks like in the wild. Ive only ever experienced it on CR.SE. Would love to get to know more.

    – bruglesco
    2 hours ago
















3















I currently spend a good deal of time over at Code Review and I would love to improve the quality of the reviews I write. Can you give me any insight into the structure or approach you use when writing a Code Review (be it on Stack Exchange or otherwise.) While I do understand that professional code reviews aren't going to look exactly the same as those on Stack Exchange, I would appreciate a perspective into how you would structure one professionally.



Where do you start? Architecture and design? Algorithm? Seriousness of flaw?



Do you structure it in any meaningful way?



Do you weigh in on controversial "best practices" topics that may not be overtly wrong in this use case?



Do you provide links to articles or Stack Overflow Questions and Answers to support your review?



How far do you go to solve the problem?



What have I missed?










share|improve this question























  • It may help if you link to a review that you have made so we can provide specific tips on things you can improve.

    – linksassin
    2 hours ago











  • @linksassin That strikes me as a critique request so I hesitate to do that. If interested though just look at my answers on CR.SE in my profile.

    – bruglesco
    2 hours ago











  • Fair enough, that's a decent reason not too. An example of the type of Code Review you are talking about might be good though. For instance code reviews conducted in git merge tools look very different to pure text formal reviews. Or are you mainly just focused on the Code Review stack answers?

    – linksassin
    2 hours ago











  • @linksassin I'm not really sure. I don't know what it looks like in the wild. Ive only ever experienced it on CR.SE. Would love to get to know more.

    – bruglesco
    2 hours ago














3












3








3


1






I currently spend a good deal of time over at Code Review and I would love to improve the quality of the reviews I write. Can you give me any insight into the structure or approach you use when writing a Code Review (be it on Stack Exchange or otherwise.) While I do understand that professional code reviews aren't going to look exactly the same as those on Stack Exchange, I would appreciate a perspective into how you would structure one professionally.



Where do you start? Architecture and design? Algorithm? Seriousness of flaw?



Do you structure it in any meaningful way?



Do you weigh in on controversial "best practices" topics that may not be overtly wrong in this use case?



Do you provide links to articles or Stack Overflow Questions and Answers to support your review?



How far do you go to solve the problem?



What have I missed?










share|improve this question














I currently spend a good deal of time over at Code Review and I would love to improve the quality of the reviews I write. Can you give me any insight into the structure or approach you use when writing a Code Review (be it on Stack Exchange or otherwise.) While I do understand that professional code reviews aren't going to look exactly the same as those on Stack Exchange, I would appreciate a perspective into how you would structure one professionally.



Where do you start? Architecture and design? Algorithm? Seriousness of flaw?



Do you structure it in any meaningful way?



Do you weigh in on controversial "best practices" topics that may not be overtly wrong in this use case?



Do you provide links to articles or Stack Overflow Questions and Answers to support your review?



How far do you go to solve the problem?



What have I missed?







technical-writing software-documentation criticism feedback






share|improve this question













share|improve this question











share|improve this question




share|improve this question










asked 2 hours ago









bruglescobruglesco

781220




781220













  • It may help if you link to a review that you have made so we can provide specific tips on things you can improve.

    – linksassin
    2 hours ago











  • @linksassin That strikes me as a critique request so I hesitate to do that. If interested though just look at my answers on CR.SE in my profile.

    – bruglesco
    2 hours ago











  • Fair enough, that's a decent reason not too. An example of the type of Code Review you are talking about might be good though. For instance code reviews conducted in git merge tools look very different to pure text formal reviews. Or are you mainly just focused on the Code Review stack answers?

    – linksassin
    2 hours ago











  • @linksassin I'm not really sure. I don't know what it looks like in the wild. Ive only ever experienced it on CR.SE. Would love to get to know more.

    – bruglesco
    2 hours ago



















  • It may help if you link to a review that you have made so we can provide specific tips on things you can improve.

    – linksassin
    2 hours ago











  • @linksassin That strikes me as a critique request so I hesitate to do that. If interested though just look at my answers on CR.SE in my profile.

    – bruglesco
    2 hours ago











  • Fair enough, that's a decent reason not too. An example of the type of Code Review you are talking about might be good though. For instance code reviews conducted in git merge tools look very different to pure text formal reviews. Or are you mainly just focused on the Code Review stack answers?

    – linksassin
    2 hours ago











  • @linksassin I'm not really sure. I don't know what it looks like in the wild. Ive only ever experienced it on CR.SE. Would love to get to know more.

    – bruglesco
    2 hours ago

















It may help if you link to a review that you have made so we can provide specific tips on things you can improve.

– linksassin
2 hours ago





It may help if you link to a review that you have made so we can provide specific tips on things you can improve.

– linksassin
2 hours ago













@linksassin That strikes me as a critique request so I hesitate to do that. If interested though just look at my answers on CR.SE in my profile.

– bruglesco
2 hours ago





@linksassin That strikes me as a critique request so I hesitate to do that. If interested though just look at my answers on CR.SE in my profile.

– bruglesco
2 hours ago













Fair enough, that's a decent reason not too. An example of the type of Code Review you are talking about might be good though. For instance code reviews conducted in git merge tools look very different to pure text formal reviews. Or are you mainly just focused on the Code Review stack answers?

– linksassin
2 hours ago





Fair enough, that's a decent reason not too. An example of the type of Code Review you are talking about might be good though. For instance code reviews conducted in git merge tools look very different to pure text formal reviews. Or are you mainly just focused on the Code Review stack answers?

– linksassin
2 hours ago













@linksassin I'm not really sure. I don't know what it looks like in the wild. Ive only ever experienced it on CR.SE. Would love to get to know more.

– bruglesco
2 hours ago





@linksassin I'm not really sure. I don't know what it looks like in the wild. Ive only ever experienced it on CR.SE. Would love to get to know more.

– bruglesco
2 hours ago










2 Answers
2






active

oldest

votes


















2














How you structure a code review depends on the tools you're using and the level of scrutiny that was requested. Instead of giving you an exact template, therefore, I'll address the different types of content.



At the lowest level, a code review can include feedback on individual lines or sections of code. Most important is to point out any errors you find; you might also point out undesired consequences (side effects, performance problems, etc). Think of this as the fine-toothed-comb level.



At the next level up, you might comment on some common themes -- concurrency, asynchronicity, pounding the snot out of the server, whatever. These are the kinds of issues you want to talk about broadly instead of pointing out every single place in the code where it happens, so you need to both explain the problem and give the review-ee enough information to be able to find and fix the individual occurrences.



At the highest level, you might have feedback not about the code itself but the design or architectural approach. Ideally there won't be too many of these because you'll have had design reviews earlier on, but sometimes stuff comes up. In deciding how to provide feedback here, ask yourself: how likely is it that anybody can actually act on what I'm going to say? Sure, this would have been better if you'd used a completely different framework or made these fundamental design decisions differently, but if that's not going to change now, there's no point in spending time on it. The purpose of a code review is to improve that code, not wish that completely different code had been written. That said, if there is an opportunity to advance (rather than ditching things and starting over), you probably do want to point that out -- e.g. the upcoming version of this library you're using is going to have feature X, which we could use to simplify this part of your code.



Now, how to present it? The lowest level is best done with some sort of code markup, like that provided by code-review tools. (I'm thinking of, for example, the git diff on pull requests here.) The other feedback, the stuff not tied so deeply into the code, is in my experience best done in a single higher-level document -- which could just be a text file -- that accompanies the code-level comments. This higher-level document might be anywhere from a few paragraphs to several pages, depending on the amount and importance of the code, the magnitude of the issues, and the level of detail requested. Try to establish some shared expectations about this beforehand; getting a tome when you expected a handful of comments or the reverse is going to be frustrating for the recipient.






share|improve this answer































    2














    It depends on the context



    Code reviews can be done for various reasons and the way to write one it heavily dependent on the purpose. Some of the reasons you might write a code review:




    • As part of a Software Quality Process


      • As a formal deliverable to management or customers

      • As quality assurance measure



    • As an answer on Code Review Stack Exchange

    • As an academic assessment or grading of one

    • As part of a hiring process for applicant submitted code


    There are a bunch more but those are some of the more common ones. The first point are the kind found most commonly in industry. I don't have experience in all of them and I won't pretend to be an expert on writing them. However here is my advice.



    Writing as a formal deliverable



    When you are writing a code review as a formal deliverable it is the same as any other report you would write. This should be a version controlled document with formal headers, footers, table of contents, etc... The goal of this kind of review is to prove that best practices have been followed and the product mets company standards. This document will be archived and kept for a long time.



    I don't know of any publicly available structure for this kind of document but most companies will have their own format that needs to be followed. In general you are looking for:




    • Code formatting

    • Error handling

    • Quality and number of testcases

    • Adherence to best practice or company standards

    • Quality of documentation


    Writing as a quality assurance measure



    The most common situation for this type of review is on change-requests to a code base. Other developers will review the code to assess if it should be merged into the main branch. I have always conducted these reviews through tools such as Github or Bitbucket. These tools provide line by line comparisons to the existing codebase and allow you to attach comments directly to the code in question.



    The style for these reviews is less of a formal review but more similar to a social media post. You can comment on anything you think needs to be changed. "Why did you do this?", "You missed a const here", "Rename this variable to X", and "Please add documentation to this function, I don't understand it" are all examples of the kind of comment you would expect in these reviews.



    These reviews are transient in nature as the developer can upload fixes to your concerns and also post replies to explain things. It is about ensuring only good code gets merged, not about writing an amazing report.



    Writing for CR.SE



    I have no expertise in this area so I will just link to Code Review's help centre on How to Write a Good Answer. The advice there can be summarised to; be insightful, organise your thought, do the best job you can and be polite.



    Writing for academia



    Most homework assignments will fall into a very similar category to 'Writing as a formal deliverable' and you should follow the same advice. When conducting a code review as part of a marking exercise you will usually have the 'correct' answer with you. It is likely no one else will ever see this review unless their marks get audited, write it however you like so long as you give a fair assessment.



    Writing as part of a hiring process



    This is similar to writing a review as part of a marking process. One key different is that you are specifically looking for skills the developer is strong in and areas where they need work. Your review should attempt to assess the skill level and experience of the applicant. This will be shared with the hiring manager and/or other members of the hiring panel. Keep it professional in case is it ever seen by anyone else.



    General Tips



    These tips apply to all types of reviews.





    • Only comment on inconsistent code formatting. Do not bother with personal style unless you are working on the same codebase and have a style guide. Tabs vs Spaces isn't an argument worth having on every review.


    • Look for bugs, typos and edge cases. Give detailed comments anytime you find an actual execution problem with the code. Potentially suggest a solution if you can identify it.


    • Back up your critiques with references. Any time you call out something that is bad practice but functional you should provide a reference to why it isn't optimal. This isn't about your opinion vs theirs so you only want to suggest actual meaningful improvements. These will have articles or guidelines that you can references in your critique.


    • Don't write their code for them. Sometimes is can be tempting to suggest a complete change just because it is not the way you would have done it. That isn't the purpose of a code review. If there are good reasons to change the approach you should explain them but this is their code. Let them solve the problem their way, point out flaws but don't attempt to solve the problem and explain it to them until their code matches yours.






    share|improve this answer























      Your Answer








      StackExchange.ready(function() {
      var channelOptions = {
      tags: "".split(" "),
      id: "166"
      };
      initTagRenderer("".split(" "), "".split(" "), channelOptions);

      StackExchange.using("externalEditor", function() {
      // Have to fire editor after snippets, if snippets enabled
      if (StackExchange.settings.snippets.snippetsEnabled) {
      StackExchange.using("snippets", function() {
      createEditor();
      });
      }
      else {
      createEditor();
      }
      });

      function createEditor() {
      StackExchange.prepareEditor({
      heartbeatType: 'answer',
      autoActivateHeartbeat: false,
      convertImagesToLinks: false,
      noModals: true,
      showLowRepImageUploadWarning: true,
      reputationToPostImages: null,
      bindNavPrevention: true,
      postfix: "",
      imageUploader: {
      brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
      contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
      allowUrls: true
      },
      noCode: true, onDemand: true,
      discardSelector: ".discard-answer"
      ,immediatelyShowMarkdownHelp:true
      });


      }
      });














      draft saved

      draft discarded


















      StackExchange.ready(
      function () {
      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fwriting.stackexchange.com%2fquestions%2f42639%2fmethods-for-writing-a-code-review%23new-answer', 'question_page');
      }
      );

      Post as a guest















      Required, but never shown

























      2 Answers
      2






      active

      oldest

      votes








      2 Answers
      2






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes









      2














      How you structure a code review depends on the tools you're using and the level of scrutiny that was requested. Instead of giving you an exact template, therefore, I'll address the different types of content.



      At the lowest level, a code review can include feedback on individual lines or sections of code. Most important is to point out any errors you find; you might also point out undesired consequences (side effects, performance problems, etc). Think of this as the fine-toothed-comb level.



      At the next level up, you might comment on some common themes -- concurrency, asynchronicity, pounding the snot out of the server, whatever. These are the kinds of issues you want to talk about broadly instead of pointing out every single place in the code where it happens, so you need to both explain the problem and give the review-ee enough information to be able to find and fix the individual occurrences.



      At the highest level, you might have feedback not about the code itself but the design or architectural approach. Ideally there won't be too many of these because you'll have had design reviews earlier on, but sometimes stuff comes up. In deciding how to provide feedback here, ask yourself: how likely is it that anybody can actually act on what I'm going to say? Sure, this would have been better if you'd used a completely different framework or made these fundamental design decisions differently, but if that's not going to change now, there's no point in spending time on it. The purpose of a code review is to improve that code, not wish that completely different code had been written. That said, if there is an opportunity to advance (rather than ditching things and starting over), you probably do want to point that out -- e.g. the upcoming version of this library you're using is going to have feature X, which we could use to simplify this part of your code.



      Now, how to present it? The lowest level is best done with some sort of code markup, like that provided by code-review tools. (I'm thinking of, for example, the git diff on pull requests here.) The other feedback, the stuff not tied so deeply into the code, is in my experience best done in a single higher-level document -- which could just be a text file -- that accompanies the code-level comments. This higher-level document might be anywhere from a few paragraphs to several pages, depending on the amount and importance of the code, the magnitude of the issues, and the level of detail requested. Try to establish some shared expectations about this beforehand; getting a tome when you expected a handful of comments or the reverse is going to be frustrating for the recipient.






      share|improve this answer




























        2














        How you structure a code review depends on the tools you're using and the level of scrutiny that was requested. Instead of giving you an exact template, therefore, I'll address the different types of content.



        At the lowest level, a code review can include feedback on individual lines or sections of code. Most important is to point out any errors you find; you might also point out undesired consequences (side effects, performance problems, etc). Think of this as the fine-toothed-comb level.



        At the next level up, you might comment on some common themes -- concurrency, asynchronicity, pounding the snot out of the server, whatever. These are the kinds of issues you want to talk about broadly instead of pointing out every single place in the code where it happens, so you need to both explain the problem and give the review-ee enough information to be able to find and fix the individual occurrences.



        At the highest level, you might have feedback not about the code itself but the design or architectural approach. Ideally there won't be too many of these because you'll have had design reviews earlier on, but sometimes stuff comes up. In deciding how to provide feedback here, ask yourself: how likely is it that anybody can actually act on what I'm going to say? Sure, this would have been better if you'd used a completely different framework or made these fundamental design decisions differently, but if that's not going to change now, there's no point in spending time on it. The purpose of a code review is to improve that code, not wish that completely different code had been written. That said, if there is an opportunity to advance (rather than ditching things and starting over), you probably do want to point that out -- e.g. the upcoming version of this library you're using is going to have feature X, which we could use to simplify this part of your code.



        Now, how to present it? The lowest level is best done with some sort of code markup, like that provided by code-review tools. (I'm thinking of, for example, the git diff on pull requests here.) The other feedback, the stuff not tied so deeply into the code, is in my experience best done in a single higher-level document -- which could just be a text file -- that accompanies the code-level comments. This higher-level document might be anywhere from a few paragraphs to several pages, depending on the amount and importance of the code, the magnitude of the issues, and the level of detail requested. Try to establish some shared expectations about this beforehand; getting a tome when you expected a handful of comments or the reverse is going to be frustrating for the recipient.






        share|improve this answer


























          2












          2








          2







          How you structure a code review depends on the tools you're using and the level of scrutiny that was requested. Instead of giving you an exact template, therefore, I'll address the different types of content.



          At the lowest level, a code review can include feedback on individual lines or sections of code. Most important is to point out any errors you find; you might also point out undesired consequences (side effects, performance problems, etc). Think of this as the fine-toothed-comb level.



          At the next level up, you might comment on some common themes -- concurrency, asynchronicity, pounding the snot out of the server, whatever. These are the kinds of issues you want to talk about broadly instead of pointing out every single place in the code where it happens, so you need to both explain the problem and give the review-ee enough information to be able to find and fix the individual occurrences.



          At the highest level, you might have feedback not about the code itself but the design or architectural approach. Ideally there won't be too many of these because you'll have had design reviews earlier on, but sometimes stuff comes up. In deciding how to provide feedback here, ask yourself: how likely is it that anybody can actually act on what I'm going to say? Sure, this would have been better if you'd used a completely different framework or made these fundamental design decisions differently, but if that's not going to change now, there's no point in spending time on it. The purpose of a code review is to improve that code, not wish that completely different code had been written. That said, if there is an opportunity to advance (rather than ditching things and starting over), you probably do want to point that out -- e.g. the upcoming version of this library you're using is going to have feature X, which we could use to simplify this part of your code.



          Now, how to present it? The lowest level is best done with some sort of code markup, like that provided by code-review tools. (I'm thinking of, for example, the git diff on pull requests here.) The other feedback, the stuff not tied so deeply into the code, is in my experience best done in a single higher-level document -- which could just be a text file -- that accompanies the code-level comments. This higher-level document might be anywhere from a few paragraphs to several pages, depending on the amount and importance of the code, the magnitude of the issues, and the level of detail requested. Try to establish some shared expectations about this beforehand; getting a tome when you expected a handful of comments or the reverse is going to be frustrating for the recipient.






          share|improve this answer













          How you structure a code review depends on the tools you're using and the level of scrutiny that was requested. Instead of giving you an exact template, therefore, I'll address the different types of content.



          At the lowest level, a code review can include feedback on individual lines or sections of code. Most important is to point out any errors you find; you might also point out undesired consequences (side effects, performance problems, etc). Think of this as the fine-toothed-comb level.



          At the next level up, you might comment on some common themes -- concurrency, asynchronicity, pounding the snot out of the server, whatever. These are the kinds of issues you want to talk about broadly instead of pointing out every single place in the code where it happens, so you need to both explain the problem and give the review-ee enough information to be able to find and fix the individual occurrences.



          At the highest level, you might have feedback not about the code itself but the design or architectural approach. Ideally there won't be too many of these because you'll have had design reviews earlier on, but sometimes stuff comes up. In deciding how to provide feedback here, ask yourself: how likely is it that anybody can actually act on what I'm going to say? Sure, this would have been better if you'd used a completely different framework or made these fundamental design decisions differently, but if that's not going to change now, there's no point in spending time on it. The purpose of a code review is to improve that code, not wish that completely different code had been written. That said, if there is an opportunity to advance (rather than ditching things and starting over), you probably do want to point that out -- e.g. the upcoming version of this library you're using is going to have feature X, which we could use to simplify this part of your code.



          Now, how to present it? The lowest level is best done with some sort of code markup, like that provided by code-review tools. (I'm thinking of, for example, the git diff on pull requests here.) The other feedback, the stuff not tied so deeply into the code, is in my experience best done in a single higher-level document -- which could just be a text file -- that accompanies the code-level comments. This higher-level document might be anywhere from a few paragraphs to several pages, depending on the amount and importance of the code, the magnitude of the issues, and the level of detail requested. Try to establish some shared expectations about this beforehand; getting a tome when you expected a handful of comments or the reverse is going to be frustrating for the recipient.







          share|improve this answer












          share|improve this answer



          share|improve this answer










          answered 1 hour ago









          Monica CellioMonica Cellio

          15k23381




          15k23381























              2














              It depends on the context



              Code reviews can be done for various reasons and the way to write one it heavily dependent on the purpose. Some of the reasons you might write a code review:




              • As part of a Software Quality Process


                • As a formal deliverable to management or customers

                • As quality assurance measure



              • As an answer on Code Review Stack Exchange

              • As an academic assessment or grading of one

              • As part of a hiring process for applicant submitted code


              There are a bunch more but those are some of the more common ones. The first point are the kind found most commonly in industry. I don't have experience in all of them and I won't pretend to be an expert on writing them. However here is my advice.



              Writing as a formal deliverable



              When you are writing a code review as a formal deliverable it is the same as any other report you would write. This should be a version controlled document with formal headers, footers, table of contents, etc... The goal of this kind of review is to prove that best practices have been followed and the product mets company standards. This document will be archived and kept for a long time.



              I don't know of any publicly available structure for this kind of document but most companies will have their own format that needs to be followed. In general you are looking for:




              • Code formatting

              • Error handling

              • Quality and number of testcases

              • Adherence to best practice or company standards

              • Quality of documentation


              Writing as a quality assurance measure



              The most common situation for this type of review is on change-requests to a code base. Other developers will review the code to assess if it should be merged into the main branch. I have always conducted these reviews through tools such as Github or Bitbucket. These tools provide line by line comparisons to the existing codebase and allow you to attach comments directly to the code in question.



              The style for these reviews is less of a formal review but more similar to a social media post. You can comment on anything you think needs to be changed. "Why did you do this?", "You missed a const here", "Rename this variable to X", and "Please add documentation to this function, I don't understand it" are all examples of the kind of comment you would expect in these reviews.



              These reviews are transient in nature as the developer can upload fixes to your concerns and also post replies to explain things. It is about ensuring only good code gets merged, not about writing an amazing report.



              Writing for CR.SE



              I have no expertise in this area so I will just link to Code Review's help centre on How to Write a Good Answer. The advice there can be summarised to; be insightful, organise your thought, do the best job you can and be polite.



              Writing for academia



              Most homework assignments will fall into a very similar category to 'Writing as a formal deliverable' and you should follow the same advice. When conducting a code review as part of a marking exercise you will usually have the 'correct' answer with you. It is likely no one else will ever see this review unless their marks get audited, write it however you like so long as you give a fair assessment.



              Writing as part of a hiring process



              This is similar to writing a review as part of a marking process. One key different is that you are specifically looking for skills the developer is strong in and areas where they need work. Your review should attempt to assess the skill level and experience of the applicant. This will be shared with the hiring manager and/or other members of the hiring panel. Keep it professional in case is it ever seen by anyone else.



              General Tips



              These tips apply to all types of reviews.





              • Only comment on inconsistent code formatting. Do not bother with personal style unless you are working on the same codebase and have a style guide. Tabs vs Spaces isn't an argument worth having on every review.


              • Look for bugs, typos and edge cases. Give detailed comments anytime you find an actual execution problem with the code. Potentially suggest a solution if you can identify it.


              • Back up your critiques with references. Any time you call out something that is bad practice but functional you should provide a reference to why it isn't optimal. This isn't about your opinion vs theirs so you only want to suggest actual meaningful improvements. These will have articles or guidelines that you can references in your critique.


              • Don't write their code for them. Sometimes is can be tempting to suggest a complete change just because it is not the way you would have done it. That isn't the purpose of a code review. If there are good reasons to change the approach you should explain them but this is their code. Let them solve the problem their way, point out flaws but don't attempt to solve the problem and explain it to them until their code matches yours.






              share|improve this answer




























                2














                It depends on the context



                Code reviews can be done for various reasons and the way to write one it heavily dependent on the purpose. Some of the reasons you might write a code review:




                • As part of a Software Quality Process


                  • As a formal deliverable to management or customers

                  • As quality assurance measure



                • As an answer on Code Review Stack Exchange

                • As an academic assessment or grading of one

                • As part of a hiring process for applicant submitted code


                There are a bunch more but those are some of the more common ones. The first point are the kind found most commonly in industry. I don't have experience in all of them and I won't pretend to be an expert on writing them. However here is my advice.



                Writing as a formal deliverable



                When you are writing a code review as a formal deliverable it is the same as any other report you would write. This should be a version controlled document with formal headers, footers, table of contents, etc... The goal of this kind of review is to prove that best practices have been followed and the product mets company standards. This document will be archived and kept for a long time.



                I don't know of any publicly available structure for this kind of document but most companies will have their own format that needs to be followed. In general you are looking for:




                • Code formatting

                • Error handling

                • Quality and number of testcases

                • Adherence to best practice or company standards

                • Quality of documentation


                Writing as a quality assurance measure



                The most common situation for this type of review is on change-requests to a code base. Other developers will review the code to assess if it should be merged into the main branch. I have always conducted these reviews through tools such as Github or Bitbucket. These tools provide line by line comparisons to the existing codebase and allow you to attach comments directly to the code in question.



                The style for these reviews is less of a formal review but more similar to a social media post. You can comment on anything you think needs to be changed. "Why did you do this?", "You missed a const here", "Rename this variable to X", and "Please add documentation to this function, I don't understand it" are all examples of the kind of comment you would expect in these reviews.



                These reviews are transient in nature as the developer can upload fixes to your concerns and also post replies to explain things. It is about ensuring only good code gets merged, not about writing an amazing report.



                Writing for CR.SE



                I have no expertise in this area so I will just link to Code Review's help centre on How to Write a Good Answer. The advice there can be summarised to; be insightful, organise your thought, do the best job you can and be polite.



                Writing for academia



                Most homework assignments will fall into a very similar category to 'Writing as a formal deliverable' and you should follow the same advice. When conducting a code review as part of a marking exercise you will usually have the 'correct' answer with you. It is likely no one else will ever see this review unless their marks get audited, write it however you like so long as you give a fair assessment.



                Writing as part of a hiring process



                This is similar to writing a review as part of a marking process. One key different is that you are specifically looking for skills the developer is strong in and areas where they need work. Your review should attempt to assess the skill level and experience of the applicant. This will be shared with the hiring manager and/or other members of the hiring panel. Keep it professional in case is it ever seen by anyone else.



                General Tips



                These tips apply to all types of reviews.





                • Only comment on inconsistent code formatting. Do not bother with personal style unless you are working on the same codebase and have a style guide. Tabs vs Spaces isn't an argument worth having on every review.


                • Look for bugs, typos and edge cases. Give detailed comments anytime you find an actual execution problem with the code. Potentially suggest a solution if you can identify it.


                • Back up your critiques with references. Any time you call out something that is bad practice but functional you should provide a reference to why it isn't optimal. This isn't about your opinion vs theirs so you only want to suggest actual meaningful improvements. These will have articles or guidelines that you can references in your critique.


                • Don't write their code for them. Sometimes is can be tempting to suggest a complete change just because it is not the way you would have done it. That isn't the purpose of a code review. If there are good reasons to change the approach you should explain them but this is their code. Let them solve the problem their way, point out flaws but don't attempt to solve the problem and explain it to them until their code matches yours.






                share|improve this answer


























                  2












                  2








                  2







                  It depends on the context



                  Code reviews can be done for various reasons and the way to write one it heavily dependent on the purpose. Some of the reasons you might write a code review:




                  • As part of a Software Quality Process


                    • As a formal deliverable to management or customers

                    • As quality assurance measure



                  • As an answer on Code Review Stack Exchange

                  • As an academic assessment or grading of one

                  • As part of a hiring process for applicant submitted code


                  There are a bunch more but those are some of the more common ones. The first point are the kind found most commonly in industry. I don't have experience in all of them and I won't pretend to be an expert on writing them. However here is my advice.



                  Writing as a formal deliverable



                  When you are writing a code review as a formal deliverable it is the same as any other report you would write. This should be a version controlled document with formal headers, footers, table of contents, etc... The goal of this kind of review is to prove that best practices have been followed and the product mets company standards. This document will be archived and kept for a long time.



                  I don't know of any publicly available structure for this kind of document but most companies will have their own format that needs to be followed. In general you are looking for:




                  • Code formatting

                  • Error handling

                  • Quality and number of testcases

                  • Adherence to best practice or company standards

                  • Quality of documentation


                  Writing as a quality assurance measure



                  The most common situation for this type of review is on change-requests to a code base. Other developers will review the code to assess if it should be merged into the main branch. I have always conducted these reviews through tools such as Github or Bitbucket. These tools provide line by line comparisons to the existing codebase and allow you to attach comments directly to the code in question.



                  The style for these reviews is less of a formal review but more similar to a social media post. You can comment on anything you think needs to be changed. "Why did you do this?", "You missed a const here", "Rename this variable to X", and "Please add documentation to this function, I don't understand it" are all examples of the kind of comment you would expect in these reviews.



                  These reviews are transient in nature as the developer can upload fixes to your concerns and also post replies to explain things. It is about ensuring only good code gets merged, not about writing an amazing report.



                  Writing for CR.SE



                  I have no expertise in this area so I will just link to Code Review's help centre on How to Write a Good Answer. The advice there can be summarised to; be insightful, organise your thought, do the best job you can and be polite.



                  Writing for academia



                  Most homework assignments will fall into a very similar category to 'Writing as a formal deliverable' and you should follow the same advice. When conducting a code review as part of a marking exercise you will usually have the 'correct' answer with you. It is likely no one else will ever see this review unless their marks get audited, write it however you like so long as you give a fair assessment.



                  Writing as part of a hiring process



                  This is similar to writing a review as part of a marking process. One key different is that you are specifically looking for skills the developer is strong in and areas where they need work. Your review should attempt to assess the skill level and experience of the applicant. This will be shared with the hiring manager and/or other members of the hiring panel. Keep it professional in case is it ever seen by anyone else.



                  General Tips



                  These tips apply to all types of reviews.





                  • Only comment on inconsistent code formatting. Do not bother with personal style unless you are working on the same codebase and have a style guide. Tabs vs Spaces isn't an argument worth having on every review.


                  • Look for bugs, typos and edge cases. Give detailed comments anytime you find an actual execution problem with the code. Potentially suggest a solution if you can identify it.


                  • Back up your critiques with references. Any time you call out something that is bad practice but functional you should provide a reference to why it isn't optimal. This isn't about your opinion vs theirs so you only want to suggest actual meaningful improvements. These will have articles or guidelines that you can references in your critique.


                  • Don't write their code for them. Sometimes is can be tempting to suggest a complete change just because it is not the way you would have done it. That isn't the purpose of a code review. If there are good reasons to change the approach you should explain them but this is their code. Let them solve the problem their way, point out flaws but don't attempt to solve the problem and explain it to them until their code matches yours.






                  share|improve this answer













                  It depends on the context



                  Code reviews can be done for various reasons and the way to write one it heavily dependent on the purpose. Some of the reasons you might write a code review:




                  • As part of a Software Quality Process


                    • As a formal deliverable to management or customers

                    • As quality assurance measure



                  • As an answer on Code Review Stack Exchange

                  • As an academic assessment or grading of one

                  • As part of a hiring process for applicant submitted code


                  There are a bunch more but those are some of the more common ones. The first point are the kind found most commonly in industry. I don't have experience in all of them and I won't pretend to be an expert on writing them. However here is my advice.



                  Writing as a formal deliverable



                  When you are writing a code review as a formal deliverable it is the same as any other report you would write. This should be a version controlled document with formal headers, footers, table of contents, etc... The goal of this kind of review is to prove that best practices have been followed and the product mets company standards. This document will be archived and kept for a long time.



                  I don't know of any publicly available structure for this kind of document but most companies will have their own format that needs to be followed. In general you are looking for:




                  • Code formatting

                  • Error handling

                  • Quality and number of testcases

                  • Adherence to best practice or company standards

                  • Quality of documentation


                  Writing as a quality assurance measure



                  The most common situation for this type of review is on change-requests to a code base. Other developers will review the code to assess if it should be merged into the main branch. I have always conducted these reviews through tools such as Github or Bitbucket. These tools provide line by line comparisons to the existing codebase and allow you to attach comments directly to the code in question.



                  The style for these reviews is less of a formal review but more similar to a social media post. You can comment on anything you think needs to be changed. "Why did you do this?", "You missed a const here", "Rename this variable to X", and "Please add documentation to this function, I don't understand it" are all examples of the kind of comment you would expect in these reviews.



                  These reviews are transient in nature as the developer can upload fixes to your concerns and also post replies to explain things. It is about ensuring only good code gets merged, not about writing an amazing report.



                  Writing for CR.SE



                  I have no expertise in this area so I will just link to Code Review's help centre on How to Write a Good Answer. The advice there can be summarised to; be insightful, organise your thought, do the best job you can and be polite.



                  Writing for academia



                  Most homework assignments will fall into a very similar category to 'Writing as a formal deliverable' and you should follow the same advice. When conducting a code review as part of a marking exercise you will usually have the 'correct' answer with you. It is likely no one else will ever see this review unless their marks get audited, write it however you like so long as you give a fair assessment.



                  Writing as part of a hiring process



                  This is similar to writing a review as part of a marking process. One key different is that you are specifically looking for skills the developer is strong in and areas where they need work. Your review should attempt to assess the skill level and experience of the applicant. This will be shared with the hiring manager and/or other members of the hiring panel. Keep it professional in case is it ever seen by anyone else.



                  General Tips



                  These tips apply to all types of reviews.





                  • Only comment on inconsistent code formatting. Do not bother with personal style unless you are working on the same codebase and have a style guide. Tabs vs Spaces isn't an argument worth having on every review.


                  • Look for bugs, typos and edge cases. Give detailed comments anytime you find an actual execution problem with the code. Potentially suggest a solution if you can identify it.


                  • Back up your critiques with references. Any time you call out something that is bad practice but functional you should provide a reference to why it isn't optimal. This isn't about your opinion vs theirs so you only want to suggest actual meaningful improvements. These will have articles or guidelines that you can references in your critique.


                  • Don't write their code for them. Sometimes is can be tempting to suggest a complete change just because it is not the way you would have done it. That isn't the purpose of a code review. If there are good reasons to change the approach you should explain them but this is their code. Let them solve the problem their way, point out flaws but don't attempt to solve the problem and explain it to them until their code matches yours.







                  share|improve this answer












                  share|improve this answer



                  share|improve this answer










                  answered 1 hour ago









                  linksassinlinksassin

                  344212




                  344212






























                      draft saved

                      draft discarded




















































                      Thanks for contributing an answer to Writing Stack Exchange!


                      • Please be sure to answer the question. Provide details and share your research!

                      But avoid



                      • Asking for help, clarification, or responding to other answers.

                      • Making statements based on opinion; back them up with references or personal experience.


                      To learn more, see our tips on writing great answers.




                      draft saved


                      draft discarded














                      StackExchange.ready(
                      function () {
                      StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fwriting.stackexchange.com%2fquestions%2f42639%2fmethods-for-writing-a-code-review%23new-answer', 'question_page');
                      }
                      );

                      Post as a guest















                      Required, but never shown





















































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown

































                      Required, but never shown














                      Required, but never shown












                      Required, but never shown







                      Required, but never shown







                      Popular posts from this blog

                      Knooppunt Holsloot

                      Altaar (religie)

                      Gregoriusmis