nullreference.dev

Things to do before send first pull request to review

Oct 2021

Code review is a good practice for any developer. And it definitely does not matter how many years of experience you have to participate in code review as a developer or reviewer person.

It helps to keep code clean, well written and stable. In addition, it will force additional knowledge transfer about functionality from more experienced persons.

Here described basic action items for new developers before sending a pull request to review.

Unfortunately more than half of the code reviews have different reasons to be postponed or rejected because of simple mistakes or misunderstandings.

The idea is to describe simple steps to increase code review productivity and as a result focus on more valuable topics to review.

For the first time, let me start from the basics.

Rebuild entire solution

By the end of the task implementation, you should rebuild entire solution and not affected projects only. Use Rebuild Solution command on context of all projects including unit-tests.

This will help you to locate any side effects caused by code changes. Also, it will force your Visual Studio or any other IDE to commit all updates to .sln and .csproj files.

Fix compiler warning and errors

Verify analyzer warnings and errors by using Run Code Analysis On Solution command. Try to fix all warnings if they appear in code affected by you.

Especially pay attention to warnings such as Possible Null Reference or Possible Multiple Enumeration. This will save you a lot of time and nerves eventually.

Unit-tests

Unit-tests or any other kind of tests is a great way to verify your code by different set of input parameters. Also, your code will be more like self-documented.

Any newly-implemented feature should be well covered by various positive and negative scenarios written in tests.

Do not be lazy to put some new tests even for small code fix. Some issues can come back later if this place is not secured by corresponding test.

By the end of the work run all existing unit-tests. It is expected to be a 100% success rate.

Review by yourself first

Double-check all commits before creating a pull request. It might happen you miss something or modify files that are not related to this pull request at all. Maybe you add new file by mistake and forgot to remove it.

Clean up any code related to debugging activities such as Stopwatch or Console.WriteLine messages. Including debugger instructions in JavaScript or any code stubs in C# code or configuration file changes.

Check words spelling. Any IDE spelling extension is more than enough for that thing.

Double-check if feature (or bug) you working on is fully implemented (fixed) by your manual tests. You should check your implementation using positive and negative scenarios.

Create pull request

Review each file in request one more time. You should clearly understand the rationale behind every changed line of code.

Provide additional comments for non-trivial code places, it will help the reviewer to understand your decision without any extra discussions.

[ back to home ]