Skip to content

Latest commit

 

History

History
112 lines (87 loc) · 5.81 KB

CONTRIBUTING.md

File metadata and controls

112 lines (87 loc) · 5.81 KB

Contributing to Provisioning

Commit messages

Make sure your commit message follows this subject style:

    type: brief summary up to 70 characters
    Refs: HMS-XXX

OR

    type(HMS-XXX): brief summary up to 70 characters

Type must be one of the following:

  • build: Changes that affect the build system
  • ci: Changes to our CI configuration files and scripts
  • docs: Documentation only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that neither fixes a bug nor adds a feature
  • chore: A non-code change that does not fit to any category
  • test: Adding missing tests or correcting existing tests
  • revert: A commit revert

The scope must be HMS Jira issue.

For feat and fix types a Jira issue link is required. Please use the scope like feat(HMS-XXX): subject or put the issue reference in commit body as Fixes: HMS-XXX or Refs: HMS-XXX

To check commit message locally please install https://github.com/RHEnVision/changelog and then run make check-commits.

Basic guidelines for code contributions

Here are few points before you start contributing:

  • Binaries go into the cmd/name package.
  • All code go into the internal/ package.
  • Binaries must not take any arguments.
  • All configuration is done via configs/local.yml and can be overwritten by environment variables.
  • Database models (structs) do belong into internal/models. Type names are named in singular form.
  • Database DDL SQL goes into internal/db/migrations as SQL files. Table names are in plural form.
  • Do not put any code logic into database models.
  • All database operations (CRUD, eager loading) lives in internal/dao (Data Access Objects) with a common API interface.
  • The actual implementation lives in internal/dao/pgx package, all operations are passed with Context and errors are wrapped.
  • Database models must be not exposed directly into JSON API, use internal/payloads package to wrap them.
  • Business logic (the actual code) does belong into internal/services package, each API call should have a dedicated file.
  • HTTP routes go into internal/routes package.
  • HTTP middleware go into internal/middleware package.
  • Monitoring metrics are in internal/metrics package.
  • Use the standard library context package for context operations.
  • Database connection is at internal/db, HTTP service clients are in internal/clients.
  • Do not introduce utils or tools common packages.
  • Keep the line of sight (happy code path).
  • Postgres version we currently use in production is v14+ so take advantage of all modern SQL features.
  • Use BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY for primary keys. This allows us to use 1-100 in our seed data (see above).
  • Use TEXT for string columns, do not apply any limits in the DB: https://wiki.postgresql.org/wiki/Don%27t_Do_This#Text_storage
  • When modifying the API endpoints or types, please update the openapi spec file as well
  • In case the change of code requires an update of AWS permissions, it is necessary to incorporate such change in different places for our app:

Keep security in mind: https://github.com/RedHatInsights/secure-coding-checklist

Upgrade safety

All changes need to be implemented with zero downtime upgrades in mind. There will be for a brief moment version N and version N+1 containers running over a single database during upgrade.

This affects migration policies. All migrations need to be backward compatible with code running without the migration. If there is a need for a breaking change, it needs to happen over course of two deployments.

Common examples:

  • Change a column name

    1. First Merge/Deploy PR that would
      1. introduce the new column nullable with default to NULL.
      2. change the code to use this new column and the old one as fallback in case first is NULL
    2. Merge/deploy a followup PR to:
      1. Migrate all data to the new column where there are data in old column
      2. Drop the fallback to the old column
      3. Drop the now unused column
  • Introduce non-nullable column without default value

    1. Merge/Deploy PR that would
      1. Introduce the column nullable with NULL default
      2. Adjust code to be adding data to the new column
    2. Merge/Deploy followup PR that would
      1. migrate data for the empty rows of the column
      2. changed the column to non-nullable
  • Drop table/column

    1. Merge/Deploy PR that would
      1. Stop using the said table/column
    2. Merge/Deploy followup PR
      1. With migration dropping the table/column
  • Change in job arguments

    • When new field is added, make sure the code is ready to process data when the value is unset (zero or nil)
    • Rename or delete is similar to migrations, there can be jobs enqueued by the old version so code must account for that.
    • Alternatively, "version" flag can be added to the arg struct for easier handling of these issues.

Testing

All code logic has unit test coverage. We test all logic in isolation