The blogpost needs an introduction: type of comment.
Conventional Comments
61–70 of 74 posts
Re: Conventional Comments
#62I'm guessing this is about Code Review comments? I always assume my comments on others' PRs (and other people's comments on my Code Reviews) are "suggestions (non-blocking)", unless indicated otherwise . Shouldn't that be the default, really?
If you use gitlab for code reviews, it's actually the opposite. Each comment thread prevents a merge unless you hit the "Resolve thread" button. It's configurable, but AFAIR it's Gitlab's default. Even though I also often use Github and Gerrit at my current job, everywhere we observe that convention that a comment is considered blocking unless indicated otherwise. This is because ~80% are typically blocking.
I personally try to encourage both communication and responsibility. So I comment on pretty much everything I don't understand/like/agree with (i.e. over-communication, that's really the only way to ensure adequate communication because by over-communicating according to your standards you're more likely hitting the other person's communication standards, which are necessarily different from your own and fundamentally unknowable), but at the same time I encourage the other person to decide which comments they want to respond to (either by arguing or by changing the code), hence showing I trust them and encouraging their personal responsibility.
Re: Conventional Comments
#63Only tangentially relevant: I hate the word 'nitpick' (bad experience with small insects - thinking about nits makes me itch), but I don't know any other word for these "small, trivial, but necessary changes." Does anyone have alternative words they use in reviews instead?
papercut?
Re: Conventional Comments
#64I get the ones where you're suggesting alterations to the code, but having "praise: You did great here" or "chore: Could you run the tests" sounds weirdly robotic to me
The issue is that many people seem to take code review comments extremely personally and get very upset and defensive.
Meanwhile, we rarely get upset at automated eslint warnings, compiler errors or issues found by Google PageSpeed Insights.
Sounding robotic prevents you from sounding offensive.
Re: Conventional Comments
#65I get the ones where you're suggesting alterations to the code, but having "praise: You did great here" or "chore: Could you run the tests" sounds weirdly robotic to me
Though specifically for "praise:", I'm a much bigger fan of the :+1: thumbs up emoji instead which accomplishes about the same thing.
Re: Conventional Comments
#66Only tangentially relevant: I hate the word 'nitpick' (bad experience with small insects - thinking about nits makes me itch), but I don't know any other word for these "small, trivial, but necessary changes." Does anyone have alternative words they use in reviews instead?
I wouldn't say nitpicks are necessary changes in most circumstances - they are really soft suggestions about very fine details, I tend to describe them with optional non-blocking comments. Nitpick is a really good term for them but "aesthetic" might work as an alternative, there isn't a huge glut of words in that realm though.
Re: Conventional Comments
#67The blogpost needs an introduction: type of comment.
"Introduction" isn't really a tone or a descriptor that changes how you act on the information like the rest.
I think but I'm not sure that's what js8 meant too.
Re: Conventional Comments
#68Earlier quoted context omitted.
If you use gitlab for code reviews, it's actually the opposite. Each comment thread prevents a merge unless you hit the "Resolve thread" button. It's configurable, but AFAIR it's Gitlab's default. Even though I also often use Github and Gerrit at my current job, everywhere we observe that convention that a comment is considered blocking unless indicated otherwise. This is because ~80% are typically blocking.
Interesting, so "passive-agressive" is already built-in. Meaning even commenting in a very innocuous style "I'm not sure if this is correct according to the style guide..." would prevent a merge. I personally try to encourage both communication and responsibility. So I comment on pretty much everything I don't understand/like/agree with (i.e. over-communication, that's really the only way to ensure adequate communica…
The proposal of conventional comments simply clarifies intention and disambiguates it. It's like that salt and pepper question. How do you know which is in which? Regardless of your knowledge and assumptions, it depends on whoever filled them in. This clarifies that.
Re: Conventional Comments
#69This document is inspired by Conventional Commits but lacks the equivalent Specification section. The point of adding structure to unstructured text is specifying the syntax required to make it machine readable. Maybe I’m missing something.
Re: Conventional Comments
#70Earlier quoted context omitted.
Disclaimer: I am a coworker of the author. I like Praise. We don’t give it often enough and if someone does something awesome like simplifying complicated tests, removing dependencies or works smart and not hard, it is due. While the conventional comments give a nice framework, nothing stops someone from adding a cheerful gif to the body of that praise. Even on larger reviews it is nice if you scroll through 10 nitpi…
I would strongly dislike being a recipient of this "praise". The thought of all of my co-workers following this rule when reviewing my merge requests makes my skin crawl. You can see from the rest of the thread that other people have a negative reaction to it too. It's nice that you enjoy it but the same does not hold true for everybody. Is there a way for somebody working with you opting out from it or are they give…