DEV Community: Marco Patiño López

Stop Writing Confusing Code Review Comments!

Marco Patiño López — Tue, 29 Apr 2025 10:09:52 +0000

We've all been there. Staring at a GitHub pull request comment like:

"Maybe change this?"

Or:

"This looks wrong, fix it."

What does that actually mean? Is it a minor suggestion or a requirement? Is it critical blocker, or just something the reviewer found confusing?
Written feedback often loses crucial context, leading to:

⏳ Wasted Time, slower reviews: Back-and-forth clarification cycles.
😠 Frustration: For both the reviewer and the author.

This ambiguity isn't just annoying; it actively harms the code review process.

Bringing Clarity with Conventional Comments

There's a better way! The Conventional Comments specification provides a simple structure to add explicit context to your review comments. It suggests prefixing comments with labels like:

praise: For highlighting good work.
suggestion: For optional improvements.
issue: For problems that likely need addressing.
question: When seeking clarification.
thought: For reflections or ideas not requiring action.
chore: For minor non-code tasks (like updating docs).

You can even add decorators for more nuance, like issue(blocking): to signal something must be fixed before merging.

Suddenly, the guesswork disappears!

issue(blocking): Add null check for 'user' before this line.

suggestion: Consider renaming 'x' to 'userCount' for clarity.

Now that's actionable feedback! Everyone understands the intent and severity instantly. It also opens the door for better tooling and tracking around code reviews.

Making Conventional Comments Effortless on GitHub ✨

Okay, using Conventional Comments clearly leads to better reviews. But how do you easily integrate this standard into your daily GitHub workflow without adding friction or needing to memorize all the labels?

That's exactly why we at Pullpo built the Conventional Comments Extension - a free, open-source Chrome extension designed to make adopting this standard effortless right within the GitHub UI.

We wanted a tool that felt native, didn't get in the way, and made applying these helpful prefixes as simple as clicking a button.

What it does:

✅ Adds an intuitive toolbar right above GitHub comment boxes (PRs, issues, commits).
✅ Provides clickable buttons for all standard labels (issue, suggestion, praise, etc.) and decorators (blocking, non-blocking, if-minor).
✅ Automatically formats the comment prefix for you – just click and type!
✅ Includes an optional "Prettify" mode that displays prefixes as neat visual badges using Shields.io (linked to a simple explainer). Looks great in PR descriptions too!
✅ Adapts seamlessly to GitHub's light and dark themes.

No more typing suggestion(non-blocking): – just click, click, type.

Give it a Try!

We built this initially for our own team, but we think it can help anyone spending time reviewing code on GitHub. It's completely free and MIT licensed.

Ready to ditch confusing comments and make your code reviews faster and clearer?

🚀 Install from the Chrome Web Store:
https://chromewebstore.google.com/detail/gelgbjildgbbfgfgpibgcnolcipinmlp
⭐ Check out the code (and star us!) on GitHub:
https://github.com/pullpo-io/conventional-comments

👀 Watch a quick demo video:
https://youtu.be/jLzXlZ78rNE?si=KMzIH9Vb43glekEW

We'd love to hear what you think! Does this solve a pain point for you? Any features you'd like to see? Let us know in the comments below!

Happy coding, and happier reviewing!

Linking Individual Engineering metrics to Business Metrics (But not the other way around)

Marco Patiño López — Wed, 26 Feb 2025 12:26:52 +0000

Every once in a while, new Pullpo users ask us to provide more individual metrics in the product. The thing is, what individual metrics? And why?

Input to Output to Outcome

After having +100 conversations with managers and engineering leaders, we saw that it's useful to divide metrics into three sets:

Input metrics, or, in the SPACE framework, activity metrics. This is what developers do and spend their time on. Number of PRs, commits, code review comments, time spent on meetings, code review turnaround time, PR size, amount of documentation written, number of LoCs… These are examples of activity metrics. They are the easy ones to measure at the individual level.
Output metrics. These start to be team-level metrics. Here we have things like the DORA metrics: MTTR, CFR, lead time, deployment frequency... and similar ones like uptime, number of incidents, support tickets, API performance…
Outcome metrics. These are business metrics. The ones that really matter. Churn, ARR, CAC… This kind of metrics reflects business impact.

The metrics that actually matter are output metrics and especially outcome metrics. So the question is, can we tie individual metrics to outcome and output metrics?

Tying individual metrics to business metrics.

Classical example: sales team.

They have all kinds of metrics to measure individual productivity like # calls made, % call to demo, % demo to close... and they work great. Why? Because they can easily link individual metrics to outcome metrics, and most importantly, the other way around: they can easily know the contribution of each team member to outcome metrics like MRR.

They can clearly measure the performance for each individual. Perfect.

What happens in engineering? We can link input metrics to output metrics, and output metrics to outcome metrics. For example, we know that a good PR Size has an impact on Cycle time (small PRs lead to faster and better code reviews), and a short Cycle time allows the company to ship faster. This means more features to promote and easier to get new clients (less CAC, more ARR) and can also reduce churn.

Ok, now try to do it the other way around. What’s the impact on CAC, ARR, and churn because a developer created a PR of the right size? Good luck.

Here is an example of tying input metrics to output metrics to outcome metrics, also inspired by this post by Iccha Sethi.

Comparing individual metrics.

Ok, so we cannot clearly tie outcome metrics to input metrics. Can we at least compare between input metrics?

Let's take the sales team example again. Imagine that we couldn't link outcome to input, at least we could compare input metrics between SDRs, right? The more calls they make (quantity) and the better close rate they have (quality), the better they are performing. Even if we couldn’t tie outcomes to inputs, even if we couldn’t know the contribution of each SDR to MRR, we could still clearly rank the performance of SDRs by only taking input metrics into account.

Ok, let's try that with engineering. So... what are quantity and quality in this case?

Quantity could be # PRs, # commits, # words of docs written, # code review comments... but how do we measure quality for those things?

In code, there are different versions of what quality means. I really like the TRUCE framework for software quality. In a nutshell, this framework analyzes the quality of the code in five areas (with different priorities depending on the stage of the business):

Timely delivery of features.
Robustness of code (reliable, tested, secure, scales, etc.).
User needs are met (meets user, customer, stakeholder requirements/needs).
Collaboration is enabled (readable, documented code to facilitate collaboration).
Evolvable design.

Here comes the trick: those things depend on more than 1 person.

Timely delivery: also depends on reviewers (and many more people in bigger companies).
Robustness, evolvable and readable code: on an existing irregular codebase, with tech debt in different files, created by many other developers.
Meets user requirements: entire product team deciding what to build.

So individual metrics are bad?

No. They are just numbers. Data is data. It's not good or bad, it is what it is.

If a manager exclusively uses activity metrics to compare individual developer productivity, then that is bad. But it's not the metrics' fault.

Can individual metrics be useful?

That's a better question. And the answer is yes, they are useful. Especially for detecting outliers in the team.

Maybe you cannot compare individual productivity of developers comparing # PRs or # commits, but it's useful to know if someone didn't create a PR or commit in the last two weeks.

In that case, your job as a leader would be to combine that info with the actual context on what's happening (maybe they are pairing, on vacation or temporarily working on another repo). And make decisions taking everything into account.

This is why Pullpo detects activity outliers automatically so that managers are aware if something is happening on their team ;)

Final thoughts and summary.

We can tie input to output and output to outcome.
However, unlike other teams, we cannot measure how much an engineer contributed to outcome.
Most of the time, it doesn't even make sense to compare developers based on input. But input metrics are useful for detecting outliers.
I say most of the time because I think there may be some exceptions to this. For example, I think review turnaround time can be a better comparative metric, allowing managers to go a little further than only detecting outliers. But not too much, or Goodhart's law could apply.
I also believe that, with AI, comparative metrics could perhaps be a solvable problem, but the solution has to be perfect. Otherwise, the "solution" would be a bigger problem than the initial problem.

The hidden cost of code reviews

Marco Patiño López — Thu, 17 Oct 2024 15:50:53 +0000

As DevOps engineers, developers, and tech leaders, there are only three things that should matter to us: Quality, Speed, and the ability to maintain both over the long term (LQT - Long-term Quality and Throughput).

Think about it—everything we do as a software development team is an investment in at least one of these categories:

Quality means building something that users want—something that works, solves the problem, and meets stakeholders’ requirements, including security and scalability.

Well-structured, maintainable, and clean code doesn’t fall under “Quality” but fits into the category of “being able to maintain both for a long time”, LQT.
If you accumulate tech debt, lack testing or documentation, you’ll eventually have to sacrifice either quality (something will break) or speed (spend time refactoring and documenting old code). Documentation, linting, managing tech debt, and fostering a strong company culture all contribute to this category and indirectly support both quality and speed.

Speed means what it means. Ship fast. Short time to market. Design, code, review and deploy.

The amount of investing into one or another aspect depends on the business stage and market situation. But you cannot ever leave one of them completely behind. Usually larger and older companies tend to focus more on quality and LQT while early stage companies prioritize quality and speed.

Every metric and framework in the space (like DORA, DevEx, and Space) also falls into one of these categories: Cycle time (Speed), MTTR (Quality), CFR (Quality), Feedback loops (Speed), Cognitive load (LQT), Diffs per engineer (Speed), etc.

Note: I think it’s also useful to correlate financial metrics like ACV, or churn & LTV to measure quality.

Every tool we make or buy should work to improve one or more aspects of this equation: CI/CD (Speed), Load balancers (Stability of the system) (Quality), Documentation (LQT), Copilot (Speed), Linters (LQT).

In essence, every tool and process in the software development lifecycle (SDLC) should contribute to Quality, Speed, or Long-term Quality and Throughput (LQT).

While most tools and processes primarily focus on one or two of these aspects, there is one critical process that significantly impacts all three simultaneously: code reviews.

Code reviews: The single process of SDLC that impacts the three categories at the same time.

Speed

On average, every PR spends 39% of its time in the code review stage. So this is an example of what leaders might see when checking their team’s engineering metrics in Pullpo’s dashboard:

And this doesn't mean that devs spend 2 days straight doing code reviews nonstop. But since it's an asynchronous process, it gets blocked while devs are doing other tasks.
Sometimes there is no problem with that, but when the code review notification and collaboration system is bad, devs stop prioritizing teamwork and unblocking others over individual tasks. And that comes with a huge cost to our ability to ship things fast as a team.

→ How we created a solution for slow code reviews.

Quality

This is the first thing that comes to our mind when we think about code reviews. Does this meet the task requirements? Will this work for all our users in all different edge cases? Are there any bugs? Will it work at scale?

In some cases, it is useful to involve non-technical people in the review process. For instance, some teams may open draft pull requests to make sure everything aligns with the UI design requirements. This is one reason why Pullpo’s PR-Channels on Slack are useful, as it enables additional stakeholders to join the conversation.

LQT - Long term quality and throughput.

There are two aspects related to LQT:

First, during code reviews, we ensure the code is well-organized, tested, documented, and stylistically correct. This contributes to LQT by guaranteeing speed and quality for the long term.
Second, the way feedback is given is crucial. Code reviews—whether async or pairing—are where most knowledge transfer happens between developers. It's also where the most feedback is given, greatly impacting engineering culture and LQT.

Using conventional comments, asking questions instead of making accusations, resolving back-and-forths efficiently, and making a habit of praising colleagues for good work—all of these practices help improve LQT.

Final thoughts.

Code reviews are the single process that affects all three of the critical areas of any software development team.
Measuring those areas can be tricky, but useful.
Improving some parts of the process can have little cost but great return.

If you want to meet the founders and chat about the product, reach out! We love to do that :)

PRs in Trunk Based Development.

Marco Patiño López — Fri, 28 Jun 2024 13:23:15 +0000

Wait, PRs in Trunk Based Development? Yes, PRs in Trunk Based Development.

There is a common misunderstanding with TBD, which is that it always implies committing directly to main/trunk and pair programming. This is false.

I talk a lot about code reviews, devex, branching models and everything around them, as I’m the cofounder at pullpo.to/phunt.

One common feedback that I’ve got many times is:

Code reviews through pull requests were made for open source. In proprietary software, we should do trunk based development (implying pair programming and committing directly to trunk)

I don’t totally agree with this claim. I’m going to explain why. To give a little bit of context…

What is TBD (Trunk Based Development)?

Basically, what TBD says is that we should avoid creating long-lived branches other than the main branch (trunk). Developers should push new commits directly there or merge short-lived feature branches (will come back to this later). The only rule for every new push or merge to trunk is to not break the build. The CI must pass.

The main concept behind TBD, which I love, is that we should keep developers close to a production environment or its closest equivalent, the main branch. Compared to other obsolete branching methods like Git Flow, TBD increases ownership and responsibility among developers, which is good. It also allows us to be always ready to deploy, and as a consequence, it normally increases deployment frequency. Since we can reach production faster, it also substantially reduces cycle time and time to recover from failures.

All of this is great, but keep reading please, the interesting part is about to come:

PRs and code reviews in TBD.

If you are committing directly to trunk, the only way to do a code review before the code gets to trunk is by pair programming. There is no alternative. If you want to do a code review (and, most of the times, you want to do a code review) you need to do pair programming.

But TBD is also compatible (and actually recommends) doing PRs and short-lived feature branches. I know some people who wont believe this, so I’m attaching an screenshot from the official TBD website. https://trunkbaseddevelopment.com/short-lived-feature-branches/

TBD also recommends pairing, as it has a lot of advantages: better communication, shorter feedback loops… The thing is, pair programming doesn’t work for everyone, all the time, for every task.

In my case, I love pairing for problems that require creative solutions, or when debugging, or when onboarding a new developer. But I often feel much more productive when working alone. Many times I cannot achieve flow state while pairing.

Other people are always super productive while pairing, for every type of task, and they love it.

We are humans. Humans are different.

If you create short-lived feature branches now you have both options for code reviews: pair programming and conventional code reviews. You can detect good opportunities for pairing and go for it. In the other cases, go for conventional code reviews.

Other benefit of creating short-lived branches:

Before pushing a new commit to a branch, first you have to pull the latest version of that branch. Depending on different factors this command can take time.

Now imagine 50 devs working on the same repo constantly pushing commits to trunk. It may be difficult to find a time window where no one pushes and you can pull the latest version and push your code.

This problem is solved with PRs and merge queues. GitHub has its own merge queue feature.

My take.

If everyone in your team is happy with always doing pair programming AND this is not a problem for hiring new devs AND you don’t have issues with the CI or with finding a long enough time window where you can pull and push with anyone changing trunk, then great! Pairing and committing to trunk is the best solution. If this condition is not met…
Do short-lived feature branches, but they have to be short-lived. Short-lived means max 2 days and only one committer (1 dev or 1 pair programming duo). Code review channels can help keep PRs fast. This is important. Otherwise, you are not doing TBD.
Learn how to identify good opportunities for pairing in your team. This is a trial and error process. Every person is different. Know yourself.
Set up you CI to run on PRs. Pair programming duos can have the option to directly merge (or send to merge queue) to trunk if the CI passes. Individual devs may need 1 or 2 approvals before merging.

.
.

I’m offering code review workshops at companies.
If you liked this post and want to implement best practices around code reviews at your workplace, let me know! I’m offering workshops and consultancy services around code reviews.

Send an email to marco @ pullpo.io

If you didn’t like the post, let me know too! I love discussing about this topics. I’ll be paying attention to all the comments and emails.

I analyzed code review best practices for a year. This is what I learned.

Marco Patiño López — Mon, 06 May 2024 09:19:38 +0000

Over the past year, while building Pullpo, I've been highly focused on one specific area: code reviews. I've analyzed and interviewed dozens of teams of varying sizes, industries, and countries to truly understand the best practices surrounding code reviews.

Why?

Code reviews are important.

Many people think that code reviews are only for improving the code, guaranteeing that the requirements for different stakeholders are met, it may be security, maintainability, performance or only functionality, depending on the environment.

But code reviews is not only about improving the code, they are many times the best opportunity of improvements and new learnings for developers inside the team.

Also, recent research (DORA 2023 report) has confirmed that improving code reviews can speed up software delivery performance up to 50%. Take a look at my summary of the report here.

Code reviews should be FAST.

This is crucial. Typically, unless you use something like SHIP, SHOW & ASK (which I recommend), all code reviews are blocking by default.

This is, until the changes meet all the requirements and get the necessary approvals, the code is not going to production.

This is something super important: We should optimize for the speed at which a team can ship product together, not for the speed at which a developer can write code. In fact, this is the first phrase of Google’s engineering practices document.

This translates to replying to code review messages as fast as possible.

Code reviews shouldn’t break your FLOW.

Being in the flow state is that time when you are super focused on a task: it can be designing, programming, whatever you are doing. Some DevEx frameworks (such as the one presented in the paper: Devex: What Actually Drives Productivity) place the flow state as a top priority subject for developer productivity.

The devex triangle from the paper DevEx: What actually drives productivity?

Flow state is very important for the efficiency of the group, so if you are a developer and you are in flow state, try to keep yourself in that state as long as you can, avoid switching tasks and as long as you can, avoid interruptions.

Being in flow state is the only reason to pospone replying fast to code review messages.

I love this image I found on reddit about the dilema between flow state and interruptions.

Credits to nkukard.

Communication is IMPORTANT.

Normally code reviews are made in a written format. Humans communicate much worse in written format. Your corporal movements are not there. Your voice tone is not there. So misunderstandings are much more common.

This is why some new communication frameworks are raising such as conventional comments. Similar to conventional commits, but applied to code review comments. Where you explicitly specify the intention behind your comments. For example:

question (non-blocking): At this point, does it matter which thread has won?
Maybe to prevent a race condition we should keep looping until they've all won?

Where non-blocking means that this comment is not blocking the PR from being accepted.

Some other tips in this area are:

Ask questions instead of affirmations.
Be kind.
If you have more seniority, spend time on mentoring, this pays off exponentially.
If you not agree on a specific topic with a teammate:
- Don’t take it personally.
- Listen carefully to the other explanation.
- Discuss it in person or in a call, where effective communication is easier.

PRs should be SMALL.

PRs/MRs should be as small as possible. Only one change at a time. Only one purpose at a time.

We’ve all seen the meme and know it’s true:

credits to twitter.com/iamdevloper

Large changes make async code reviews super slow if we want to keep quality, or super low quality if we want to keep speed.

So what can we do when tasks are huge and we need to implement many changes to accomplish it and we can’t divide the tasks into functional small subtasks?

Stacked pull requests. This is a great alternative. Basically it means dividing the task into reviewable (<400 locs) PRs. The downside of this approach is that since these PRs depend on each other, a change on the first branch requires updating the depending branches too.

This fact can make you lose some time synching the depending branches manually with ‘merge’ or ‘rebase’, this is why some tools where born such as git-town, ghstack or graphite.

Credits to Dr. McKayla

Pair programming. Some companies use pair programming as the default way of developing. This also means that the code is already being reviewed by 2 people while being developed, so you don’t need to do a conventional async code review afterwards.

Notes on pairing.

In the past, some recognized authors such as Dave Farley, author of Continuous Delivery, claimed a controversial idea (read comments on video or here) that code reviews through PRs are just a bad idea.

In this video, Dave proposes pair programming as the best code review solution for every case, for every type of company, every type of developer and every type of task.

I follow Dave, and I agree on most of the things he says but most of the time things are not just black or white, but somewhere in the middle depending on different factors. Pairing is one of those things.

As a developer, you need to know yourself in order to understand in which scenarios you feel more productive pairing, with which type of people and on which type of problems.

In my case, typically, I only feel productive while pairing in the following scenarios:

Mentoring a new person on the team.
Brainstorming creative solutions for a complex problem.

However, it’s very hard for me to achieve flow state while pairing, and as we discuss earlier in this post, flow state is a key factor on development productivity.

Final thoughts.

Code reviews need to happen fast, this is replying fast to code review messages.
Being in flow state must be the only reason to pospone code review messages.
Communication is important. Be nice. Listen. Discuss in person/call if necessary.
Only one purpose per PR. Small PRs. Stacked PRs if necessary.
Try pair programming with different people and tasks. Discover when pairing is useful for you (and your team).