Skip to main content

Best practices for Code Review





Benefits of Code Review

  1. Multiple eyes on a piece of code. Finding bugs early.
  2. Coding standards compliance
  3. Consistent design and implementation. Better Structured code.
  4. Team cohesion
  5. Learn how to write better code
  6. Teaching and Sharing Knowledge. Mentoring tool.

 Guidelines for Authors
Before raising PR:
- Code is "complete as possible"
- Small Pull Request.
- In case of Long Request, "Point out important parts"
- Explain changes in PR Description along with JIRA story links
- Read the checklist and mark the things which you have considered for this PR
- Pull Request Title : Use JIRA ticket and useful title description

After Raising PR:
- Be grateful for the reviewer's suggestions.
e.g. Good call. I'll make that change., Whoops. Good catch, thanks. Fixed in a4994ec.
- A common axiom is "Don't take it personally. The review is of the code, not you."
- Try to respond to every comment.

 Guidelines for Reviewers
- Don't nitpick - Focus on the correctness of code. Less focus whitespaces/variable names.
- Don't be a jerk - No harsh comments plz
- Ask good questions; don't make demands.
- Avoid selective ownership of code. ("mine", "not mine", "yours")
- Provide Examples  ( Small code snippets will do )
- Seek first to understand

Tools and General Advise:
- Schedule time for PR's
- Automate with code linting
- Use a checklist

 Pull Request Template:

 To add this template to GitHub project, You just have to create "pull-request-template.md" file in root directory.
 Below is the sample format for the PR template.

# Description

Please include a explaination of the change. Please also include relevant motivation and context. List any dependencies that are required for this change.

JIRA Ticket link: 

Please delete options that are not relevant.

- [ ] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] This change requires a documentation update

# How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

- [ ] Test A
- [ ] Test B

# Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] Unit test cases added for this change.
- [ ] Considered accessibility for UI change                                                                                                                                                                                                      - [ ] There are no merge conflicts                                                                                                                                                                                                                                                              .                                                                                                                                                                                      # Impacted Areas in Application                                                         List general components of the application that this PR will affect.


 


Comments

  1. jonathanNovember 26, 2017 at 4:05 AM

    Very good optimized process taken into the consideration for the code review methodology. You have focused every area of coding for reviewing the code logic and code sanity.

    ReplyDelete

Post a Comment

Popular posts from this blog

How to access a Android database by using a command line.

How to access a Android database by using a command line. Many of us uses databases in android applications, So it is very important to know how it is store, where it is store in the device and how to access that database directly outside from your program. That is helpful to understand whether our database code working as per expectation. Steps to do that: 1) You need to launch the emulator first. Better launch your database application from Eclipse.  ( Note: Even you can connect your real device to your PC for this. ) 2) Launch a command prompt in the android platform-tools directory. ( Directory which has adb.exe ) 3) type  adb shell . This will launch an unix shell on your emulator / connected device. 4) go to the directory where your database is : ( beware, Unix is case sensitive !! ) cd data/data here you have the list of all the applications on your device Go in your application directory  cd com.employeedirectory and descend in your databases directo...
1 comment
Read more

Protect sensitive information or credentials using Android Keystore

The Android keystore provides secure system level credential storage. With the keystore, an application creates a new Private/Public key pair, and uses this to encrypt application secrets before saving it in the private storage. We will learn how to use Android keystore to create and delete keys also how to encrypt the user sensitive data using these keys. The Keystore system is used by the  KeyChain API as well as the Android Keystore provider feature that was introduced in Android 4.3 (API level 18). This document goes over when and how to use the Android Keystore provider Android has had a system-level credential storage since Donut (1.6). Up until ICS (4.0), it was only used by the VPN and WiFi connection services to store private keys and certificates, and a public API was not available. ICS  introduced  a public  API   and integrated the credential storage with the rest of the OS.  Why to use Keystore?     ...
1 comment
Read more

Certificate and Public Key Pinning in Android

Certificate and Public Key Pinning in Android. Now a days it’s very easy for an attacker to intercept the request and responses in secured channel ( SSL / TLS ).   This allows the attacker to get in the middle of the conversation between a client and server. They could be just eavesdropping on the conversation or could be changing the data as it moves to the client or server. This kind of attack is known as “Man in the Middle Attack” (MiTM). You can read more about MiTM attack here ( https://en.wikipedia.org/wiki/Man-in-the-middle_attack ) What is Certificate Pinning or Public key pinning?                 Certificate Pinning is most often used to address the scenario above, i.e. to keep unwanted eyes from looking into a mobile application’s traffic. Certificate pinning is hardcoding or storing the information for digital certificates/public keys in a mobile application. In mobile application...
Post a Comment
Read more