#13: The Ultimate Contribution

From Junior to CTO Weekly Thoughts

In the previous issue, we discussed how The Ultimate Commit transformed to The Ultimate Artifact within The Ultimate CI Process. But one commit is not a feature on its own, we need to have a properly formatted branch, guarantee commit shape, etc. In this issue, I’ll cover this particular topic: how to do The Ultimate Contribution.

As before, most part of my experience is based on Gitlab, so I’ll show Gitlab-based examples, but you will probably easily find replacements for your specific scenario.

Let’s clarify what the Definitions of Done for The Ultimate Contribution are.

Definitions of Done

Let’s formulate rules on how The Ultimate Contribution should look like:

• every new commit should be The Ultimate Commit

• every contribution made with corporate’s email (if applicable)

• every new branch

  • should be named after the ticket code

  • should be merged only with The Ultimate Merge Request (a.k.a. Pull Request)

  • should follow The Ultimate Branching Model

• every new Merge Request

  • should follow The Ultimate Approval Process

  • should pass The Ultimate Code Review (we will discuss this in the next issue)

  • should pass The Ultimate Quality Review

  • should contain all relevant information

  • should preferably be refreshed via rebase before the merge

  • should be merged either with fast-forward or merge commit

  • as a part of a bigger feature should be merged in the right order

  • should trigger The Ultimate CI Pipeline

• squash is prohibited with exclusions

• every repo configuration should follow The Ultimate Repository Configuration

• every contribution policy should be documented

Rules are defined. Let’s see how to ensure compliance with them.

Automation of DoD Compiance Check

every new commit should be The Ultimate Commit

In The Ultimate Commit post, I mentioned that to guarantee The Ultimate Commit structure (especially Conventional Commits), we can use githooks. Technically, it is not a strict constraint because githook could be either disabled or occasionally not enabled. So local git hooks help avoid push errors and prevent them early but do not guarantee restriction. If we want to perform strict checks we have to enable push rules (gitlab naming).

Currently, this functionality of Gitlab is located in Group → Repository Settings → Pre-defined push rules or Repository → Repository Settings → Push rules

Pattern:
^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)

Some issues with Semantic Release require Gitlab 15.10+, but generally, this functionality helps guarantee the check.

every contribution made with corporate’s email (if applicable)

The same story as for the commit format. Sometimes people might contribute from personal email with a random name config. Eventually, git history looks like a coven of daemons but not real people. To protect your team from this kind of mistake, you can just set a pattern to allow only corporate emails

every new branch should be named after the ticket

Fortunately, this part is also manageable in the same place, so if you use Jira, probably, your ticket codes look like XX-1234, it means that branches should follow the same format:

^[A-Z]+-[0-9]+$

every new branch should be merged only with The Ultimate Merge Request (a.k.a. Pull Request)

The Ultimate Merge Request is a regular MR that must be implemented according to the rules below but it is important that contribution never should come directly to the default branch. You could reasonably argue: “There should be exclusions when you urgently need to push a fix directly to the default branch!”. Making this constraint weaker only because once per 1-2 quarters you see a use case doesn’t make sense because, at the same time you make your pushes error-prone, and occasionally, people will push directly even if they don’t want. I’d say that the negative impact of this situation is much heavier than the profits you get making the constraint weaker.

every new Merge Request should follow The Ultimate Approval Process

First of all, approvals must be a part of your development process. Period.

Approvals management is located in Settings / General / Merge request approvals (Gitlab, but not available for some plans, you also might use emojis).

Code Owners is another important practice, so you must get approval from the most experienced people in a particular module/package/file. Except for Code Owners, approvals must be obtained from the most relevant people, so the team or a developer should choose if there is uncertainty.

The Ultimate Approval Process is out of the scope of this issue but will be covered soon.

every new Merge Request should pass The Ultimate Code Review

Code Review is a vital part of contribution quality assessment. If you compare code review with feature testing, you see that developers should help to find bugs that are harder to find from outside because other developers see how the code works. Like testing, it is easy to perform Code Review badly, focusing on the wrong things. I’m going to cover this topic in the issue the next week, so subscribe not to miss

every new Merge Request should pass The Ultimate Quality Review

Quality review is another part of quality assessment. It is a comprehensive quality check for built artifacts rather than static sources. A quality review should include a run of automated e2e testing as well as manual feature testing, security check, etc.

every new Merge Request should contain all relevant information

Merge Request is an important point of documented collaboration between engineers. The better Merge Request is formulated, the better collaboration becomes. If this doesn’t become overcomplicated and relevant for your process, MR should contain a copy of ticket requirements not to confuse reviewers if something is changed in Jira but not in the MR yet. MR must describe the motivation for the change (context). MR description must guide a reviewer not to miss key things and contain a checklist for routine things like “permissions for new feature are configured”, “documentation is updated”, and also playing the role of reminder for the change author. Added Code Quality metrics (absolute values & diff) will help display what is going on with code quality and introduce the opportunity to configure code quality gateways.

Use templates to unify MR creation:

Sometimes commits in Merge Requests could be discussed on calls, but the results of discussions must be documented inside a commit message body or comments in the MR.

every new Merge Request should preferably be refreshed via rebase before the merge

Before we merge changes, we have to review and test them refreshed with the latest state from the default branch. There are two most popular options: rebase and back-merge. Let’s shortly compare

                        | Rebase  | Back-merge
-----------------------------------------------
Clear history           |   +     |     -
No history rewrite      |   -     |     +
Confl. resolution once  |   -     |     +

Do you think the best option is obvious? Not really. Weight might differ depending on your context.

Clear history?

Mostly about aesthetics, but if it doesn’t require extra effort and your team enjoys it, why not prefer this way? Commit graph review use case is also an argument, but honestly, ask yourself how often you look at graph? Were you really interested in graph connections or commit order after all? Can a simple search in the history (via IDE plugin) cover your use cases?

No history rewrite?

If we are talking about one developer per one feature branch - the weight here is zero, so this condition doesn’t impact because history rewrite does not introduce issues of concurrent work on the same branch. Nobody pushes changes directly to the master branch. Everyone primarily pushes changes to their feature branches.

Conflict resolution once?

If you rebase and more than one of your atomic commits might conflict with the target branch, that is the time to think about back-merge for this particular case.

Modern IDEs that drastically simplify the process must be used in any way.

What approach is the best?

You could prefer to rebase from an aesthetics and graph simplification point of view. If you see potential conflicts and back-merge will save you decades of minutes - good motivation to stop rebasing for this particular feature (branch) and start back-merging because aesthetics should not impact business.

every new Merge Request should be merged either with fast-forward merge or merge commit

This is pretty clear that fast-forward merges are the ultimate way of merging. No synthetic merge commits, and absolutely clear history (only one branch, finally). But! Fast-forward merges require you to rebase history, which is a strict constraint. So, that might be the main pain point if you apply The Ultimate Commit approach. At the same time, your team might be fine with this inconvenience. Choose either fast-forward forward merge or merge commit, get an agreement with your team, and follow only one approach, do not use two approaches simultaneously because that will confuse everyone and make your collaboration more unpredictable.

You could also apply merge commit with semi-linear history to guarantee that you merge only those changes that consider the default branch's latest state. That might work perfectly in tandem with the “approval reset on a new commit” option and approval by QA Engineer and fully automated regression testing, but you introduce more bureaucracy, and that definitely will slow down your delivery (of course, quality might keep better at the same time). Choose one depending on your particular business needs.

Do not forget to reconfigure your repositories

every new Merge Request as a part of a bigger feature should be merged in the right order

Sometimes features consist of more than one Merge Request, and the order might be important. For example, if you implement CD practices, you must merge the backend before the frontend that uses the backend. The backend should be delivered first.

The latest Gitlab version supports Merge Trains to automate the order of merge.

squash is prohibited with exclusions

In the Ultimate Contribution, squash is prohibited because your commits follow The Ultimate Commit practices, and the history is a source of extremely meaningful insides. After a while since a commit was introduced, you will review why this line looks like it is. The answer will be there.

Of course, there are no strict rules without exclusions. Dummy commits that were useful when you triggered CI especially to check that CI works fine must be squashed because they are meaningless (actually, they must be eliminated). This is relevant for feature branches that have only one responsible engineer but not for shared branches like your default one.

You also can hide “squash commits” option for MR in project settings.

every repo configuration should follow The Ultimate Repository Configuration

The more repos you have, the more complex to manage their settings manually. Especially if someone could change and forget to return them. The Ultimate Repository Configuration is out of the scope of this issue, but I’d like to emphasize that the configuration should be synced automatically and defined in a dedicated repository.

every contribution policy should be documented

I’m not an expert in open source, but more than once, I saw that popular open-source projects define their policies. I apply similar practices and maintain a few internal how-tos for my team that help people to align and onboard new teammates. It is important to simplify the onboarding process, create contribution guidelines considering agreed rules, and mention automation that helps inside the document. Team changes become seamless.

Conclusion

The Ultimate Contributions defines how to introduce new changes and not forget anything; how to form merge requests, branches, and commits.

One of the crucial parts of The Ultimate Contribution is code review. In the next issue, we will discuss The Ultimate Code Review and how to make the most of it.

The Video Of The Week

`