Code Quality and Static Code Analysis

Introduction

Maintaining code quality is a crucial aspect of software development. Ensuring our codebase is clean, maintainable, and free from errors is a collective responsibility. This document outlines our policies, best practices, tools, and metrics for maintaining the highest code quality in our project.

Tools and Technologies

• Backend (Django):

  • Linter: flake8

    • exclude = .git, .gitignore, *.pot, *.py[co], __pycache__, venv, .env, .coverage, openIMIS/wsgi.py, dist, openIMIS/migrations, ../**/migrations/*.py, # Relative paths for other modules ../**/.git, ../**/.gitignore, ../**/*.pot, ../**/*.py[co], ../**/__pycache__, ignore = # At least two spaces before inline comment. E261, # Too many blank lines (3). E303, # Ambiguous variable name. E741, # Unused imports - sometimes required in modular version, fixes in __init__.py may break builds # Ideally added for every __init__ file manually but for the time it's added globally F401 max-line-length = 120

  • Static Code Analysis: SonarQube

  • Unit Testing: Django's built-in testing tools

• Frontend (React):

  • Linter: ESLint

    • { "env": { "browser": true, "es2021": true }, "extends": [ "plugin:react/recommended", "airbnb" ], "overrides": [ ], "parserOptions": { "ecmaVersion": "latest", "sourceType": "module" }, "plugins": [ "react" ], "rules": { "react/prop-types": "off", "no-shadow": "off", // disabled due to use of bindActionCreators "react/jsx-filename-extension": [1, { "extensions": [".js", ".jsx"] }], // disabled due to naming consistency with other modules "import/no-unresolved": "off", // disable due to module architecture. For modules most references are marked as unresolved "max-len": ["error", { "code": 120 }] } }

  • Static Code Analysis: SonarQube

Workflow

1. Pre-push/Pre-PR:

   • Developers should run flake8 for Django code and ESLint for React code before pushing their changes.

   • Ensure unit tests are passing for backend code.

     • During CI/CD pipeline test execution tests for all core modules are executed, not only for single module.

2. CI/CD Pipeline:

   • Upon creating a PR or pushing to specific branches, our Github Actions CI/CD pipeline will automatically:

     • Run the linters (flake8 & ESLint)

     • Execute all backend unit tests

     • Perform static code analysis through SonarQube

3. Results Visualization:

   • Linter and unit test results are directly reported in the Github Actions CI/CD pipeline logs.

   • SonarQube results, including code coverage, code smells, and potential bugs, are visualized in our SonarQube dashboard linked with the project.

Policies and Practices

1. Code Quality Metrics:

   • All PRs should pass the default quality gates set in SonarQube.

   • Linter errors (from both flake8 and ESLint) must be addressed.

   • Backend unit tests should always pass.

   • We encourage the community to provide feedback on additional metrics and standards. Open a discussion thread or issue for suggestions.

2. Code Review Process:

   • Every PR undergoes a review by at least one core developer.

   • Address all failing checks in the CI/CD pipeline before requesting a review.

   • Reviewers should prioritize the logic, efficiency, and alignment with the project's goals.

   • SonarQube's feedback should be considered advisory. Address critical and high-severity issues. For minor issues, reviewers' discretion is advised.

3. Community Involvement:

   • Given the open-source nature of the project, community feedback on code quality metrics and practices is invaluable. Please contribute to the discussion threads or open new ones with suggestions.

Code Review

Every piece of code, irrespective of its origin, undergoes a review process:

1. Reviewer Requirement: At least one review from a team member is mandatory for every pull request.

2. Checklist for Reviewers:

   • Code follows the project's style guidelines.

   • Changes are consistent with the overarching architecture.

   • Adequate tests are in place.

   • All tests pass.

   • No direct database calls; always use the ORM.

   • Check for potential security vulnerabilities.

   • Ensure no hardcoded values; always use constants or environment variables.

   • Code is DRY (Don't Repeat Yourself).

   • Comments are clear, and no commented-out code is present.

Backend CI/CD Pipeline

All backend modules should use pipeline defined in the openimis-be_py in order to execute code analysis (Workflow Definition).

Flow executed by the CI Job:

Open

Example setup: Social Protection Module

Setup variables:

SONAR_TOKEN - either taken from organization settings or from sonar project site

SONAR_PROJECT_KEY - key from Sonar, usually it’s openimis_<repository_name>

SONAR_ORGANIZATION - organization key, always openimis-1 for modules in organization

SONAR_PROJECT_NAME: repository name

SONAR_PROJECT_VERSION: 1.0

SONAR_SOURCES: social_protection

SONAR_EXCLUSIONS: excluded directories for coverage, by default "**/migrations/**,**/static/**,**/media/**,**/tests/**"

Workflow Overview

• Trigger: The workflow is triggered through workflow_call. It can receive several optional secrets and inputs, like SONAR_TOKEN and SONAR_PROJECT_KEY, allowing for flexibility and customization.

• Jobs:

  1. build_backend: Prepares the backend for testing.

  2. ci_module_mssql_tests: Runs all tests on Microsoft SQL Server.

  3. ci_module_psql_test: Runs all tests on PostgreSQL.

  4. flake-8-linter - This job is responsible for running code style checks using Flake8 on Python code.

  5. sonar_scan - This job is responsible for running static code analysis using SonarCloud to identify code vulnerabilities, code smells, and to analyze the coverage report.

Jobs Details

build_backend

• Environment: Uses Ubuntu 20.04 and Python 3.8.

• Dependencies: Caches Python dependencies to speed up future runs.

• Cloning: Clones the openIMIS Backend from the develop branch.

• Configuration: Updates configurations using jq for JSON processing and replaces or adds the current module into openimis.json.

• Artifacts: Compresses and uploads Python site-packages and openimis code as artifacts.

ci_module_mssql_tests

• Environment: Uses Ubuntu 20.04 and Python 3.8.

• Dependencies: Downloads previously cached Python packages and installs them.

• Database Setup: Uses a Microsoft SQL Server container and initializes it with the openIMIS database.

• Testing: Runs Django tests against this SQL Server database. It utilizes environment variables to specify the database details.

ci_module_psql_test

• Environment: Similar to the ci_module_mssql_tests job but targets PostgreSQL.

• Database Setup: Uses a PostgreSQL container for the database.

• Testing: Similar to the SQL Server job but for PostgreSQL.

flake-8-linter

• Environment: Uses Ubuntu 20.04 and Python 3.8.

• Dependencies: Requires builded backend and it’s artifacts.

• Configuration: Uses .flake8 setup from assembly module. Linter is done for whole module not only new code.

sonar_scan

• Environment: Uses Ubuntu 20.04

• Dependencies: Require coverage report and flake8 report generated in previous steps.

• Configuration: Module details passed using execution variables. Quality Gateways can be configured directly on sonarCloud.

Notes on Code Quality Checks

• The use of jq ensures that the JSON configuration (openimis.json) is programmatically editable, maintaining a level of correctness.

• The use of environment variables and GitHub secrets, like SONAR_TOKEN, ensures security and confidentiality.

• The caching mechanisms enhance performance and reduce build times, aligning with best practices.

Sonar Quality Gateway

Currently all modules are using default Quality Gateway provided by Sonar. It expects code to:

• Have 80% coverage

• < 3% duplicates

• < 5% Technical Debt

• No Bugs

• No Security Vulnerability

Suggestions for further process improvements

Disagreements & Escalation

While we anticipate most discussions to be constructive, disagreements might arise. When they do:

1. Both parties should discuss their perspectives in the PR comments.

2. If no agreement is reached, a third-party core team member will intervene and provide feedback.

3. As a last resort, the project maintainers have the final say on including or excluding a piece of code.

Refactoring

As the project evolves, refactoring becomes necessary to maintain code quality:

1. Regular Refactoring Sprints: Every three months, a sprint dedicated to refactoring and addressing tech debt can be beneficial.

2. Community Involvement: While community contributions primarily focus on features and fixes, we should encourage community members to participate in refactoring. Picking up smaller refactoring tasks can be a great way to familiarize oneself with the codebase.

Module CI

Add functionality allowing passing openimis.json as call paramter. Currently it’s fixed for develop branch which will not be optimal for all builds. Ideally all modules should be executed with theirs dependencies and modules that are depending on given one in order to make sure that changes didn’t affect other modules.