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
.
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 ininternal/clients
. - Do not introduce
utils
ortools
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
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
- First Merge/Deploy PR that would
- introduce the new column nullable with default to NULL.
- change the code to use this new column and the old one as fallback in case first is NULL
- Merge/deploy a followup PR to:
- Migrate all data to the new column where there are data in old column
- Drop the fallback to the old column
- Drop the now unused column
- First Merge/Deploy PR that would
-
Introduce non-nullable column without default value
- Merge/Deploy PR that would
- Introduce the column nullable with NULL default
- Adjust code to be adding data to the new column
- Merge/Deploy followup PR that would
- migrate data for the empty rows of the column
- changed the column to non-nullable
- Merge/Deploy PR that would
-
Drop table/column
- Merge/Deploy PR that would
- Stop using the said table/column
- Merge/Deploy followup PR
- With migration dropping the table/column
- Merge/Deploy PR that would
-
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.
All code logic has unit test coverage. We test all logic in isolation