LogoLogo
  • Technical Direction
  • Technical overview
    • Technical Implementation
    • API Design Guide
      • Data Definitions and Standards
      • Data Transfer Objects
      • Documentation
      • Environments
      • Error Handling
      • Example API Service
      • GraphQL Naming Conventions
      • Methods
      • Naming Conventions
      • Once Only Principle
      • Pagination
      • Resource Oriented Design
      • REST Request
      • REST Response
      • Security
      • Versioning
    • Ísland.is Public Web Data Flow
    • Code Reviews
    • Code Standards
    • Monorepo
    • Project Management
    • Teamwork
    • Architectural Decision Records
      • Use Markdown Architectural Decision Records
      • Use NX
      • Continuous Integration
      • CSS
      • Branching and Release Strategy
      • Error Tracking and Monitoring
      • What API Management Tool to Consider
      • Viskuausan Static Site Generator
      • Use OAuth 2.0 and OpenID Connect As Protocols for Authentication and Authorization
      • Unified Naming Strategy for Files and Directories
      • CMS
      • Open Source License
      • What Chart Library Should We Use Across Island.is?
      • What Feature Flag Service/application Should We Use at Island.is?
      • Logging, Monitoring and APM Platform
      • ADR Template
    • Log Management Policy
  • Products
    • Island.is Authentication Service
      • Terminology
      • Integration Options
      • Authentication Flows
      • Authorising API Endpoints
      • Session Lifecycle
      • Scopes and Tokens
      • Delegations
      • Configuration
      • Tools and Examples
      • Environments
      • Test IAS with Postman
      • Using the IAS admin portal
    • Notifications / Hnipp
      • New Notification Setup Guide
      • Notifications service workflow overview
      • Email notifications
    • Pósthólfið
      • Security Checklist
      • Introduction
      • Skjalatilkynning API
      • Skjalaveita API
      • Sequence Diagram
      • Interfaces
    • Straumurinn (X-Road)
      • Architecture Guidelines for Service Providers and Consumers
      • Setting up an X-Road Security Server
        • Network Configuration
      • X-Road - Uppfærsla á öryggisþjónum
      • Straumurinn - Notkun og umsýsla
      • X-Road Central - current version
  • Development
    • Getting Started
    • Generating a New Project
    • Definition of done
    • Devops
      • Continuous Delivery
      • Database
      • Dockerizing
      • Environment Setup
      • Logging
      • Metrics
      • NextJS Custom Server
      • Observability
      • Operations Base Principles
      • Security
      • Service Configuration
      • Support
    • AWS Secrets
    • Feature Flags
    • Documentation Contributions
    • Defining Monorepo Boundaries With Tags
    • OpenAPI
    • Code Generation
    • Workspace Settings (Deprecated)
    • External Contributions
  • REFERENCE
    • Problems
      • 400 Validation Failed
      • 400 Attempt Failed
      • 403 Bad Subject
      • 400 500 Template API Error
    • Glossary
  • Misc
    • Guide: Adding a Payment Step to an Application
    • Guide: Enable Organisations to Make Requests to an Application
    • README Template
Powered by GitBook
On this page
  • Culture
  • Code Coverage
  • Dashboard
  • Pull request comments
  • Merge checks

Was this helpful?

  1. Technical overview

Code Reviews

PreviousÍsland.is Public Web Data FlowNextCode Standards

Last updated 1 year ago

Was this helpful?

Code reviews are a valuable tool for improving code quality and sharing knowledge across the teams working on the codebase. There are tons of best practices available online, but over time each company (or as in this case a swarm of companies working in the same codebase) adopts practices which suits their needs and culture.

Culture

Code reviews lose all their value if nobody feels motivated to comment on code that they feel needs improvements or further explaining, or if these comments have a negative effect on the programmer who committed the code.

Reviews should be concise and written in neutral language. Critique the code, not the author. When something is unclear, ask for clarification rather than assuming ignorance.

There are many programmers contributing to this repository, people who have different backgrounds, culture and personalities. It is very likely that without a common code review etiquette, people will receive comments on their pull requests that might offend them, when the commenter meant no harm, or accidentally chose some unfortunate words.

To minimize the likelihood of this happening we want to apply the following language when reviewing code committed by programmers you do not know:

  • Start comments with Consider: where you want to share knowledge, or do not have a strong opinion on if this change is necessary in order to get an approval for the pull request.

  • Start comments with Should: where the code in question should be changed because it is buggy, has some dangerous side effect or introduces a bad performance hit. These types of comments often spark a discussion, so try to keep it positive and rational. If you cannot back this comment on with meaningful arguments, you should not have made this comment to begin with.

Code Coverage

Code coverage is a measurement used to express which lines of code were executed by a test suite.

Code coverage is measured by the test runner which can be configured to generate a coverage report at the end of a test run.

Even though the coverage report can be viewed in a browser by itself it is quite limited.

To enhance the experience we use a code coverage tool called .

The main features are listed below and instructions on how to use them.

Dashboard

Pull request comments

Basically hits are lines are covered by tests, misses are lines that are not covered with tests and partials are lines partially covered with tests.

To calculate the coverage ratio we do hits / (hits + misses + partials)

Merge checks

Currently merge checks are not configured.

The shows us an aggregated coverage report of all the apps and libraries in the monorepo.

The be able to read through a Codecov pull request comment we need to understand Codecov's coverage diff and calculations regarding hits, misses and partials. See the for details.

Here is a to use as a reference while trying out these calcuations.

Codecov
Codecov dashboard
documentation
coverage diff