Is labeling Pull Requests messages a good idea?
Tags: git, github, codereview, team
Published: 2023-01-08 20:50:06

<!-- wp:paragraph -->
<p>Reviewing a Pull Request is always a challenge: how do we keep quality and code consistency while keeping developers motivated? <strong>Does labeling Pull Requests messages make things easier to interpret?</strong> </p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Recently, <a href="https://twitter.com/felipe_elia/status/1611080113545674753" target="_blank" rel="noreferrer noopener">I tweeted about the https://conventionalcomments.org/ website</a>. The idea is simple: every time you review a PR, <strong>your messages should start with a label</strong>, making it clear which is the next expected step (if any.)</p>
<!-- /wp:paragraph -->

<!-- wp:image {"align":"center","id":2566,"width":666,"height":494,"sizeSlug":"full","linkDestination":"custom"} -->
<figure class="wp-block-image aligncenter size-full is-resized"><a href="https://twitter.com/felipe_elia/status/1611080113545674753" target="_blank" rel="noreferrer noopener"><img src="https://felipeelia.com.br/wp-content/uploads/2023/01/tweet-about-conventional-comments-1.png" alt="" class="wp-image-2566" width="666" height="494"/></a><figcaption class="wp-element-caption">E aí, já me segue no <a href="https://twitter.com/felipe_elia" target="_blank" rel="noreferrer noopener">Twitter</a>?</figcaption></figure>
<!-- /wp:image -->

<!-- wp:heading -->
<h2 class="wp-block-heading">What is a <em>Pull Request</em> (or <em>Merge Request</em>?)</h2>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p><strong><em>A Pull Request</em> is a request to add some code to the project's "official" code.</strong> Depending on the tool, these requests can be called <em>Merge Requests</em>.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>It goes like this:</p>
<!-- /wp:paragraph -->

<!-- wp:list -->
<ul><!-- wp:list-item -->
<li><strong>Person A</strong>:
<ul><!-- wp:list-item -->
<li>Writes some code for a specific change or addition</li>

<li>Opens a PR describing what and why has been changed;</li>
</ul>
</li>

<!-- wp:list-item -->
<li>A revision and adequation process happens. Part of this process can depend on a <strong>Person B</strong> review.</li>


<li>The code is merged. </li>
</ul>



<!-- wp:paragraph -->
<p>This post refers to messages from <strong>Person B</strong> to <strong>Person A</strong> explaining the changes needed before the code can be merged.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p><strong>DISCLAIMER:</strong> There is a lot of material about PR etiquette. This post does not cover this specific topic but I do recommend you to search about it. Although just vaguely related, I recommend you to watch <a href="https://www.youtube.com/watch?v=jMpCF0Z623s" target="_blank" rel="noreferrer noopener">this video by Dave Farley</a> about leadership and <strong>how important it is to allow people to solve problems in ways you would not do it.</strong></p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Prefixes and labels: Agreements about the format</h2>
<!-- /wp:heading -->

<!-- wp:image {"align":"center","id":2563,"sizeSlug":"large","linkDestination":"custom"} -->
<figure class="wp-block-image aligncenter size-large"><a href="https://conventionalcomments.org/" target="_blank" rel="noreferrer noopener"><img src="https://felipeelia.com.br/wp-content/uploads/2023/01/conventional-comments-1024x437.png" alt="" class="wp-image-2563"/></a></figure>
<!-- /wp:image -->

<!-- wp:paragraph -->
<p>The website suggests the following format for revision comments on <em>Pull Requests</em>:</p>
<!-- /wp:paragraph -->

<!-- wp:preformatted -->
<pre class="wp-block-preformatted">&lt;label&gt; [decorations]: &lt;subject&gt;

[discussion]</pre>
<!-- /wp:preformatted -->

<!-- wp:paragraph -->
<p>For example:</p>
<!-- /wp:paragraph -->

<!-- wp:quote -->
<blockquote class="wp-block-quote"><!-- wp:paragraph -->
<p><strong>suggestion (test,if-minor)</strong>: It looks like we're missing some unit test coverage that the cat disappears completely.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>I am not sure if this is a big problem or not. Would it be possible to add tests for that scenario before we merge this?</p>
<!-- /wp:paragraph --></blockquote>
<!-- /wp:quote -->

<!-- wp:paragraph -->
<p>In the example we have:</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p><strong>Label (Ex.: <em>suggestion</em>):</strong><br>The <em>suggestion</em> label indicates this is about a possible improvement. What and why should be changed.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p><strong>Decorators (Ex.: <em>test, if-minor</em>):</strong><br>A quick way to expand the label meaning. The comment is about tests and the decision of whether it should block or not the merge is up to the PR author, depending on the size of the problem.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p><strong>Subject:</strong><br>The main message of the comment.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p><strong>Discussion:</strong><br>It is not required but can help to further explain the subject.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>The website has very good suggestions for <em>labels </em>and <em>decorators</em>.</p>
<!-- /wp:paragraph -->

<!-- wp:heading -->
<h2 class="wp-block-heading">Two highlights: <em>nitpick </em>and <em>praise</em></h2>
<!-- /wp:heading -->

<!-- wp:paragraph -->
<p>From the entire list, there are two that I found very interesting: <em><strong>nitpick</strong> </em>and <strong><em>praise</em></strong>.</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>The <strong style="font-style: italic;">nitpick</strong><i> </i>label is a good way to say "This is not wrong but I would do this differently". I recommend you to pay attention and not overuse this but this label can be a good way to share knowledge.</p>
<!-- /wp:paragraph -->

<!-- wp:image {"align":"center","id":2564,"sizeSlug":"large","linkDestination":"media"} -->
<figure class="wp-block-image aligncenter size-large"><a href="https://felipeelia.com.br/wp-content/uploads/2023/01/conventional-comments-nitpick.png"><img src="https://felipeelia.com.br/wp-content/uploads/2023/01/conventional-comments-nitpick-1024x154.png" alt="" class="wp-image-2564"/></a></figure>
<!-- /wp:image -->

<!-- wp:paragraph -->
<p>The <em style=""><b>praise</b></em> label is for compliments. This is one way to <strong>recognize a good, creative, and/or elegant solution</strong>. As each comment creates a thread, the team can agree that the author can "resolve" it right after reading it.</p>
<!-- /wp:paragraph -->

<!-- wp:separator -->
<hr class="wp-block-separator has-alpha-channel-opacity"/>
<!-- /wp:separator -->

<!-- wp:paragraph -->
<p>What do you think about this? Share your thoughts in the comments section below or via <a href="https://linktr.ee/felipe.elia" target="_blank" rel="noreferrer noopener">social networks</a>!</p>
<!-- /wp:paragraph -->