Git Pull Requests vs. Formal Code Reviews?

Is a pull request equivalent to a (formal Fagan) Code Review? 

The Short Story

Well... Maybe yes if you do some additional work over BB/Git. 

Out of the box: NO.

First, remember what is a pull request (and what it is good for):
https://help.github.com/articles/about-pull-requests/
https://help.github.com/articles/creating-a-pull-request/
https://help.github.com/articles/requesting-a-pull-request-review/

Why aren't they equivalent?
Because formal review procedures typically require the creation of review records (CRRs are separate - potentially deliverable - documents that identify the scope of the review, the reviewers, the subject under review and all the issues identified) and the collection of metrics that might be checked against (code review) performance baselines in order to evaluate the quality of the review being performed and feed the continuous improvement process. 

The Long Story

If you do not generate CRRs you might get into re-certification issues (difficulties showing compliance, verifiability and making the records auditable) or contractual issues (if the customer needs and/or demands you evidence of code reviews being performed).
So it is a good mechanism to improve code quality but they could not replace totally a formal [Fagan] code review unless you generate (manually or automatically with some bespoke scripting) CRRs.

INTERNAL: Projects should use a lot pull requests (since they also contribute to code quality improvements even though efficacy and efficiency are hard to attest and prove), and code reviews at Crucible (the official tool for CR) when compliance is needed internally (with the Verification Process: Large projects, Large Teams, budgets >= 125 KEUR) and/or when review records are needed for the customer or the standards of the target market.

Kudos to D. Almeida for the distinction of the 2 (apparently) similar and redundant mechanisms.

Tools: Code Reviews and Support tools

Why (NOT) Code Reviews? 

Code reviews is not the most beloved verification method for a developer (in the end of the day it makes you proof-read others code, and everyone knows that all code other than yours is not good enough - #irony). But remember that Code Reviews are one of the most cost-effective verification methods available: Fixing a bug in software while developing it is way much cheaper than fixing it after the formal testing phases begin (several studies on the subject can be found - just "google" for it).

Also, things that we should bear in mind before we start complaining about having to do code reviews are:

• There are things that are much easier to verify looking at the code directly (e.g. correct use of RDBMS transactions) than by integration / system testing.  "White box" rules here and saves us hours (as a team - because on some cases we are saving testers' time).
• For some critical parts of your code it might be a good idea to have them being checked (read: code reviewed) prior to any deployment to production, e.g., code involved in authentication, authorization, money transfers, etc..  

Code Reviews in your QMS/SDP

Your QMS/SDP might have a Verification Process defining a code review procedure on how to conduct code reviews (see "Further Reading"). And how metrics related to code reviews are to be collected (and when, and by whom). Examples are: code review speed, preferably per technology; number of defects detected, number of defects fixed, defect fix cost - e.g. in minutes, etc.

And What About Supporting Tools?

So in order to "reap the benefits" (read: improve SW Quality and save money), give the developer teams a help by supporting the procedure with tools. For instance: Crucible (a plugin to Atlassian JIRA). Or any other. Anything other than making people use printed code and / or word processor annotated documents.

Further Reading

Examples (permissions could apply; INTERNAL):

• Example of a QMS Verification Process (Supporting Processes): https://quality.critical.pt/QMS%20PT/SUP-support/SUP-03-verification/CSW-QMS-2002-PCS-0354-verification.pdf#search=crucible%20guidebook
• Example of Code Review Support Tool Guidebook: https://quality.critical.pt/QMS%20PT/SUP-support/SUP-03-verification/CSW-QMS-2002-PCS-0354-verification.pdf#search=crucible%20guidebook
• Example of a code review under Crucible: https://crucible.critical.pt/cru/ATG-5#CFR-11408

DIGEST: SW Construction - Further Reading

Additional Resources for SW Construction:

While building your software (aligned with the gathered requirements - SRS - and the software design that your high-level architecture defines - SAS) several additional topics are relevant. For instance: Coding standards to comply with (if any); comments to create and % of comments and eventual traceability to requirements IDs / component ID there left; static analysis to perform (and extracted KPIs to comply with - if any); continuous integration server and tasks to use; verification measures such as unit tests, code reviews. For more generic information on these topics see here.
   
For further detailed information please see the following links (some are INTERNAL).

Static Analysis: 

1. http://silvaonsoftware.blogspot.pt/2016/05/tools-source-monitor-static-analysis.html
2. http://silvaonsoftware.blogspot.pt/2016/05/tools-static-analysis-tools-listing.html
3. http://silvaonsoftware.blogspot.pt/2016/10/sw-construction-static-analysis-tools.html

Code Reviews: 

1. The QMS context (INTERNAL): Review Procedure here; Crucible Tool GBK here; CheckStyle (Java) tool GBK here.
2. Code Reviews example - Crucible (Plugin to JIRA; INTERNAL): https://crucible.critical.pt/cru/ATG-5#CFR-11408

Continuous Integration (CI):

1. About CI, a list of several servers available can be seen here: https://en.wikipedia.org/wiki/Comparison_of_continuous_integration_software
2. Example - Jenkins (INTERNAL): http://jenkins.critical.pt/computer/(master)/ (INTERNAL; Teamcity could also be in use for some projects).

Unit Testing: 

1. Think "developer tests" (and see Module 9 for now; again, INTERNAL)

Further reading - Coding Standards: 

Each company might have internally defined coding standards. Or it could be using external standards (e.g. HP, NASA, ESA, etc.). Google for "C Coding Standard" and see what it is meant by coding standards here: For instance this is 1994 NASA-SEL's style guide for C (PDF).

Further reading - BOOKS: 

There are lots of books on SW Engineering that focus on best practices for coding activities, with the objective of writing clearer code, easier to understand and maintain, easier to reuse, etc. 

Two must read examples might include: 

• Clean Code - Robert C. Martin, 
• Code Complete 2 - Steve McConnell

Many more books have full chapters on writing "good" code. 

Again, "Google" and/or "Amazon" for it.

Happy readings!