Merge Requests
We fixed the bug. We completed the feature. We underestimated a task and need to integrate the first batch of changes. Regardless, our changes work for us and we're ready to inflict them on others.
To prevent any vile code infecting our main branch, we scrutinize integrations into our trunk via Merge Requests (MR) - the first gate-keeping mechanism to keep our product stable. We run automated tools and automated tests and request manual and AI driven code reviews. All serious source control platforms offer automation, tooling, and UIs for creating and reviewing MRs.
Metadata
Besides quality control, a MR allows developers to provide context for their changes. We can provide information as to why the new code was necessary, attach links to tickets and bug reports, and provide any media to explain the expected behavior.
The entrypoint to any MR is its title. Our coworkers read it and our tools parse it. When we share a link to our MR, every preview and social image features the title. We generate release notes based on MR titles. It also becomes the message for our merge commit. Short, concise, and descriptive titles improve the readability and traceability of our changes.
Thus, here's a highly opinionated guide on merge request titles:
An opinionated guide on merge request titles
Writing MR titles follows the same principles as writing commit messages.
- We limit the title to 50 characters
- We capitalize the title
- We do not end the title with a period
- We use the present tense ("Add feature" not "Added feature")
- We use the imperative mood ("Move cursor to…" not "Moves cursor to…")
A properly formed MR title completes the sentence "If applied, this change will ...":
If applied, this change will Cap the number of threads for concurrent uploads ✅
If applied, this change will Capped the number of threads for concurrent uploads ❌
If applied, this change will Concurrent uploads now cap the number of threads ❌
When writing our MR title, we try to convey the most context with the least amount of characters. Thus we avoid the following practices:
- We do not clutter the title with references to external tasks, organizational tags, or flags to launch automation tools. Prefixes and suffixes decrease the legibility of our carefully crafted title.
- Instead of using the ingrained yet slothful "add", "remove", "update", or "refactor", we improve our messages by starting the title with a representative verb.
- The title conveys why the code changed, not how it changed. We do not include any reference to the implementation strategy. The code changes themselves convey that information.
| Suboptimal | Better |
|---|---|
| [bugfix][224] Update authentication flow to be streamlined | Redirect users to the visited page after sign-in |
| [JIRA-1123] Add authentication for the user with OpenID | Authenticate user via OpenID |
| [dev-tests] Refactor cache to use redis instead of mongo | Reduce latency for cached content |
With the title tabled, we supply additional details using the MR description. This field allows us to append any information we deem helpful to contextualize the code changes. While relevant documentation of code should be contained in comments in the source code itself, this section covers information too verbose for code comments and helpful to colleagues searching for the intention behind changes. We expand upon why our code changes were necessary and how we expect the behavior of our software to change after our merge.
MR descriptions can be as short as a couple sentences, or an extensive reference of our implementation strategy. Depending on the change, we may append appropriate data, such as benchmark data to showcase performance improvements, hyperlinks to our task management tool, and references to our design documentation. If we made any crucial decisions during the implementation process, we explain the pros, cons, and consequences of them and highlight what circumstances would make us revisit our solution.
The description text may also contain formatted data for tooling and automation. While modern CI platforms allow users to create and attach labels to MRs to organize and orchestrate automation tasks, these become insufficient for complex workflows. When tools rely on dynamic schemas and instructions, we may add input to the description. Depending on the notation of our format, we add data as comments or well-formed syntax at the beginning of our description. This data is only consumed by automation runners and should be invisible to human readers.
Merge constraints
MRs act as a gate-keeping mechanism to our main branch. We subject incoming changes with a healthy amount of scrutiny and examine them using automated and manual reviews to ensure we merge high-quality code. For every constraint we place on integrations, we provide context as to why that particular check exists. People tend to object less against integration criteria when we have a clear reason for introducing them (and the criteria themselves are sensible). Once a branch passes all checks, it is considered green and may be merged into our trunk.
Typical merge constraints consist of:
- Verifying conflict-free code changes: Our CI/CD system ensures the changes we made do not collide with changes made by others. Any modern version control system offers this check out-of-the-box.
- Build the software: Even if changes across developers do not collide, the union of multiple changes may not result in a compiling code. People moved imports, renamed methods, or forgot to add files to the version control system. We compile and/or build the software to verify the submitted state.
- Static Analysis: A suite of tools that inspects the code syntax without running the code. We discuss this step in detail in the next paragraph.
- Automated test runs: A set of tools that runs the software and executes a test suite to verify our changes do not break existing behavior. We discuss automated tests in detail in the chapter Testing.
- AI code review: We feed our changes to an LLM. The model highlights dubious code based on the observations of its training data.
- Manual code review: A team member inspects our changes. A fresh set of eyes may catch a mistake we made or an edge-case we missed. We discuss manual code review in detail in the chapter Code Reviews.
Static Analysis
Before we request a review, we run our static analysis tools. These tools scan our code without actually running it and verify it follows agreed-upon standards. We would struggle to find a greater waste of time and resources than manually reviewing a MR and requesting changes to improperly formatted source code, left-over debug outputs, or unused commented implementation drafts.
The most fundamental form of static analysis uses pattern matching to ensure our compliance with internally agreed-upon code formatting. We follow our organization's conventions regarding the type of whitespace, indentation, maximum line length, naming of objects, etc. We run language-specific code linters and extend these to cover the needs of our team. Maintaining a consistent code base reduces friction when integrating our work within and across our teams by reducing the cognitive load when reading code. Shared code patterns help our colleagues focus on the content of what our code, rather than the syntactical layout.
Certain problems are easier to discover when scanning the source code rather than running the compiled artifacts. Data flow analysis informs us about unused variables, uninitialized variables, and null pointer dereferences before we execute our code to discover crashes. It also highlights potential problems in parallel or asynchronous environments. Race conditions are infamously difficult to reproduce and fix in production and can be prevented to a degree with sophisticated static analysis.
Subject matter experts offer static analysis tools as a stand-in review tool to prevent common pitfalls for security, data integrity, and performance. Call graph analysis tools visualize call stacks to flag potential bottlenecks or highlight problematic flow control and error handling. Resource leak detection flags missing closures of objects, allocated memory, or network connections. These aspects severely affect the runtime stability of our system and subject us to high-security risks. Hostile actors piggyback on open database connections or memory layouts to read exposed data or inject unintended behavior.
The rise of permissive open-source code licensing spawned the liberal use of third-party packages in modern development environments. While efficient, this means we delegate the maintenance of our product to an independent party. Dependencies to outdated or compromised libraries expose attack vectors to malicious third parties. Static analysis highlights problematic dependencies to vulnerable packages before executing the code and endangering our system.
Effective static analysis tools don’t just flag problems—they offer solutions. Wherever possible, we automate the resolution of detected issues. Tools that criticize our work without further context create frustration. Once our annoyance reaches a tipping point, we disregard warnings, mute the output, and merge changes regardless. Thus, we wield static analysis as a scalpel, not a hammer, enforcing only high-impact issues and severe security vulnerabilities.
Static analysis tools provide measurable insights on their usage and results. We can track the full context of detected issues, their frequency, and the time required to resolve them. The wide range of data helps us with any source code–related organizational initiative.
For example, deprecating a widely used library in a large codebase can feel like a game of whack-a-mole. As we work to remove dependencies peppered throughout the source, a steady stream of copy-pasted code, AI-generated code, and legacy hotfixes reintroduce the imports and includes we are trying to eliminate.
Writing and deploying static analysis tools to warn engineers about the use of the deprecated library helps prevent new dependencies from spreading across the organization. Providing an official, in-place alternative to the existing implementation increases adoption of the new solution and reduces the likelihood that developers will ignore warnings and merge their changes anyway.
While we may strongly recommend the use of organization-wide static analysis tools (or even mandate one or two security tools), individual teams ultimately decide what to integrate into their merge process. To gain broad adoption across teams, our static analysis tools must be configurable and easy to tune. Noisy reports due to unproductive classifications or skewed severities will ultimately lead engineering teams to abandon the tool. As with any CI/CD tool or practice, we establish clear goals for each objective and track progress over time.
-
Good read? Unlock the rest of the chapter!
Engineering Collaboration is currently available as an Advanced Reading Copy for select readers.