Development Standards

Code review, commit message format, and development best practices

This guide outlines the standards and practices for developers contributing to the Halo measurement system.

Commit Message Format

Commit messages must strictly adhere to the Conventional Commits specification to enable tooling to parse them.

Commit Types

The following types are adopted from the Angular Commit Message Guidelines:

Type	Description
build	Changes affecting the build system or external dependencies
ci	Changes to 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 with no externally visible changes
test	Adding missing tests or correcting existing tests

Format Structure

Basic Format

<type>(<scope>): <description>

[optional body]

[optional footer(s)]

Commit Message Rules

1. Write imperative descriptions

   The description should be a complete imperative sentence:

   • ✅ “Delete the Foo method from the Bar service”
   • ❌ “Deletes the Foo method from the Bar service”
2. Use breaking change indicator

   The ! indicator must be used for breaking changes:

   feat!: Change API authentication method
   
   BREAKING-CHANGE: Old API keys will no longer work
   
3. Add Git trailers

   Format footers as valid Git trailers:

   Issue: org/repo#123
   RELNOTES: New feature for improved performance
   

Use BREAKING-CHANGE (with hyphen) instead of BREAKING CHANGE to ensure it’s a valid Git trailer.

Additional Trailers

Issue Trailer

Every significant PR should reference at least one GitHub issue:

Issue: world-federation-of-advertisers/cross-media-measurement#456

Use the short link format, not full URLs.

RELNOTES Trailer

For release note text that isn’t a breaking change:

RELNOTES: Improved query performance for large datasets

Pull Request Format

The PR description populates the commit message when merged:

• PR title → First line of commit message
• PR description body → Body and footers of commit message

Remove any line wrapping that GitHub introduces into the PR title and commit message to ensure parseability.

Code Review Process

Use Reviewable

All code reviews are conducted using Reviewable.io. Read the Reviewable user guide, especially the section on Code review discussions.

While reviews happen in Reviewable, PR merging is still done through the GitHub UI.

Review Workflow

1. Reviewer leaves comments

   The reviewer adds comments in either Blocking or non-Blocking disposition.

2. Author responds

   The author:

   • Pushes code changes
   • Responds to comments
   • Marks comments as “Satisfied” when addressed
3. Iterate until approval

   Repeat steps 1-2 until the reviewer approves. The Publish button automatically shifts to “Publish & Approve” once all blocking comments are satisfied.

Comment Dispositions

• Blocking - Must be resolved before approval
• Non-blocking - Suggestions that don’t block approval
• Satisfied - Author has addressed the comment
• Working - Author is still working on a fix (use rarely)

Authors should only publish comments when there’s an action for the reviewer to take. Wait until you’ve pushed the fix before replying.

Review Code Quickly

Prioritize keeping other engineers unblocked:

Treat as Urgent

Code review is an urgent asynchronous task. Prioritize it over design docs, coding, and addressing your own PR comments.

Avoid Needless Blocking

• Suggest adding TODOs instead of fixing non-pressing issues
• Preemptively approve straightforward fixes
• Link to citations for style/readability issues

Aim to merge PRs within 48 hours to minimize merge conflicts, reduce context switching, and simplify dependent branch management.

Code Authorship

Review Your Own Code First

Before adding reviewers:

1. Write detailed PR description

   Include references to related GitHub issues

2. Check for issues

   • Typos
   • Style guide violations
   • Insufficient documentation
3. Mark temporary changes

   Use DO_NOT_SUBMIT comments for changes that shouldn’t be merged. An automated check will block the PR.

Code Style Guidelines

The following language-specific guidelines apply to all contributions.

Pull Requests, Issues & Milestones

Pull Requests

• Every PR should be associated with an existing Issue
• If associated with an issue, the PR must have an Issue trailer in the commit message

Issues

Type

Each issue must have a type (e.g., Bug, Feature request)

Title

Titles must be self-contained and meaningful without parent context

Dependencies

Declare blocking relationships in the issue’s Relationships field

Use the appropriate issue template when creating an issue. Fall back to the ‘blank’ template if no special template applies.

Milestones

• Only issues (not PRs) should be linked to milestones
• Process issues (not related to src files) have type “Process” and aren’t associated with milestones

Code Style

Detailed code style guidelines are organized by language and topic.

General Principles

Thorough Testing

• Test the contract (public API), not implementation
• Use appropriate test doubles (fakes, mocks, stubs)
• Prefer more, smaller test cases

Code Quality

• Autoformat and lint all code
• Keep code free of compiler warnings
• Limit unnecessary third-party dependencies

Handling Compiler Warnings

1. Fix or work around

   Make a strong attempt to address the warning or find an alternative approach.

2. Suppress if false alarm

   If the code is safe and correct, suppress the warning with a comment explaining why.

3. Add TODO if needed

   For warnings that can’t be addressed yet (e.g., deprecated symbols still in use), add a TODO. Do not suppress these warnings.

Project Conventions

• Tests go in src/test/ tree
• Test infrastructure used by multiple packages goes in testing subpackages in src/main/ (marked testonly)
• Public service APIs follow AIPs with noted exceptions
• Database access is restricted to internal API servers
• Database internal IDs must not be exposed outside internal APIs
• Protobuf messages for database rows end in Details

Language-Specific Guidelines

Kotlin

• Follow Google Android Kotlin style guide
• Exception: Use 2 spaces instead of 4 for indentation
• Use Truth for test assertions
• Use ProtoTruth for protobuf assertions
• Prefer Kotlin DSL builders for protocol buffers
• Specify types explicitly except when obvious
• Use companion objects for constants
• Catch StatusException from RPCs immediately at call site

Formatting: Use ktfmt with --google-style

C++

• Follow Google C++ Style Guide
• Follow Abseil’s Tips of the Week
• Don’t rely on transitive includes
• Use include-what-you-use tool

Formatting: Use clang-format with --style=Google

Bazel BUILD and Starlark

• Follow Bazel BUILD style guide
• Follow .bzl style guide
• Don’t rely on transitive dependencies
• Include all directly referenced dependencies in deps

Formatting: Use Buildifier

Protocol Buffers

Follow Protocol Buffers style guide

Markdown

• Follow Google Markdown Style Guide
• Maintain compatibility with GitHub Flavored Markdown and Gitiles

TODOs

Use TODO comments to point out desired improvements. Each TODO should be actionable, explaining how it can be resolved.

Format

TODO(<context>): <comment>

The context indicates the person or tracked issue with the most information (not an assignee).

GitHub Issue

// TODO(world-federation-of-advertisers/cross-media-measurement#123): 
//   Switch to new API after migration

Prefer referencing issues over people whenever possible, as well-written issues provide permanent history and additional context.

Next Steps

Testing Guide

Learn about testing procedures

Contributing

How to contribute to the project