Semantic Code Reviews – Simple and direct comments without drama | Hacker News Reader

Semantic Code Reviews – Simple and direct comments without drama

m31coding.com

76 pointskmschaal46 comments

Respectful agreement: I can see how this would work well to encourage the encoding of correct tone in comments, which typically lack the ability to convey that information.

Whimsical analogy: The Elcor in Mass Effect adopted a similar approach to manage communicating with other races who lacked their ability to communicate nuance through scent and microgestures.

> Elcor speech is heard by most species as a flat, ponderous monotone. Among themselves, scent, extremely slight body movements, and subvocalized infrasound convey shades of meaning that make a human smile seem as subtle as a fireworks display. Since their subtlety can lead to misunderstandings with other species, the elcor prefix all their dialog with non-elcor with an emotive statement to clarify their tone.

Damn you made my day with the Mass Effect reference. Just happens I bought the legendary edition over the weekend to ahem get my kids into it too!

Observation: This is also similar to how HK-47 from KotOR communicates.

Cynical observation: HK-47 is commonly passive-aggressive and sarcastic.

I've been a big fan of this approach for years (introduced to me by Conventional Comments [1]) - even though it may not always be necessary, it's a low-effort way to make sure you're not misunderstood.

[1] https://conventionalcomments.org/

I'd previously tried Conventional Commits but I found the number of labels they suggest too many to keep in my head. The labels in post map well to how I think about comments.

There is a chrome plugin for this that is awesome. I started using it and one of the things i saw is that the amount PR's that I block went down significantly. Because i will classify block or non blocking per issue so don't end deciding at the end based on the amount.

So for example something that has 10 nitpicks I will let it pass because all it tells me is i'm a dick :)

> There is a chrome plugin for this that is awesome

Link?

I’ve had great luck with conventional commits by just ignoring half of them. feat, fix, chore, docs - that’s enough to cover a LOT of cases.

Thank you for sharing. The example, where the same sentence is presented with different labels, effectively illustrates the essence of the idea.

There is a chrome plugin for this that is awesome

Appreciation: Thanks for pointing out a related project. (This comment was written to adhere to Conventional Message Board Feedback v3.0.0)

Or just criticize and take criticism like its part of the job. Being perfect in it has huge costs.

The problem with this is that people are rarely given training in understanding criticism and how to deliver it constructively in industry. Maybe it's different in some computer science curriculum?

All too often, eager young developers are told to, "do code reviews," without being given any instruction on how.

This can lead to all kinds of unproductive misunderstandings, such as:

- Interrogating the author's competence, Why did you add the fields to this struct instead of...

- Shaming people, You should know... or How could you miss something so obvious? etc

- Bikeshedding, this one is subtle and a huge source of wasted effort and a huge nuisance. When discussing the construction of a nuclear power plant the most well attended and heated discussion was what color to paint the bike shed. Consider if your comment would really change anything for the better or is it just creating busy work? Remember: personal preferences aren't important, and neither are superficial ones.

Simple things you can do to avoid these kinds of comments, sound more constructive and assertive: refrain from the use of the words "I," or "you." Use the non-personal pronouns, "this," "they," to talk about the code. Stick to facts and avoid personal preference: you might have written it differently but if the tests pass, the style fits the rest of the module, there are no lint warnings, etc; people shouldn't have to come to you to ask how you would have written it. And use labels as the article suggests: if you feel compelled to leave some helpful advice for a junior colleague that isn't necessary to get the code merged, leave a label to indicate your intention.

In a similar fashion, authors: make sure you're comfortable rejecting poorly written review comments, ignore unhelpful nit-picking, etc. Practice rejecting a few nitpicks once in a while. After all, you took the time to understand the problem and write the code in the first place. Some times reviewers dropping by at the last minute haven't taken the time to fully understand and appreciate the problem. If their suggestion would have you re-write half of the work you did and not materially change outcomes it's a sign they have no idea what they're talking about.

> take criticism like its [sic] part of the job

Taking unprofessional or deconstructive criticism is not part of the job.

Suggestion: read the article

> a lot of time and energy are spent coping with the general disadvantages of written communication, such as the lack of tone and body language

If you encounter criticism that could be improved, help improve it: https://chromium.googlesource.com/chromium/src/+/master/docs...

I've worked on a team where that view was adopted, and really tried to cope. I don't miss that, at all.

After reading a round of feedback, I frequently had to make a pause, 30 minutes minimum. I tried to control my will to respond at the same tone, not always managing to hold the impulse. There was a lot of rumination and resentment too, not only on my part, but on the rest of the team's. Such a huge waste of mental energy.

Pretending that people is supposed to behave like a robot, dismissing their emotions etc, IMO is a mistake and, sometimes, an excuse used by people who feel good treating others badly.

Nice post. However, imho with a good enough writing style it should be possible to convey the purpose of the text without an explicit label.

I tend to agree with that, however don't understand the language barrier. Labeling comments makes the intent unambiguous.

I also enjoy a tactical use of smileys and other emojis where applicable :)

Argh, I meant "however don't underestimate". I blame mobile keyboards and their autocorrect features.

Thank you for your comment. Your experience leads me to believe that the concept might be more useful for non-native speakers. The labels have helped us shorten lengthy PR discussions and focus on the factual aspects. For context: I work in a German company with an international team, and we use English as our business language

This "good enough writing style" must also be able to cross cultural borders - I dare to say even journalists don't get it right most of the time:

* I've heard Kiwis say "F*ck you?" in lieu of "Seriously?"

* A Brit who says "That is an interesting solution!" usually does not feel intellectually stimulated by the solution but is conveying that it is utter garbage.

Thus, I very much like the proposed idea of well-defined labels.*

Where I come from (western US) that would be "f*ck me?" I think "interesting" has become the universal anodyne term for dismissal.

That's modern social media and even then partly.

If in work setting your coworkers (god forbid teamleads or managers) interact like that just run, don't try to solve it with conventional reviews or something. Believe me you will not finish blinking before they start misusing this too to cope with their repressed anger through mockery/shallow sarcasm. "nitpick: have you considered escaping user input?"

> A Brit who says "That is an interesting solution!" usually does not feel intellectually stimulated by the solution but is conveying that it is utter garbage.

I can be sarcastic but I have never directed sarcasm towards someone who I am reviewing. (Sarcasm towards third-party code is okay for me.)

I don't know if people who are using sarcasm would want to start labeling things like that as such, since it tends to detract from the intended effect.

Disclaimer: not a Brit.

Well yeah, but in those examples the commenters do not use direct or clear language, they use colloquialisms and passive-aggressive remarks; if you avoid those, a lot of problems with cultural difference are already resolved.

Anything that avoids ambiguity here is beneficial. Even if the comment itself is perfectly crafted, the potential for misunderstanding remains. This method outlines the expectations for responses much more than it conveys the tone of the message. This is critical information that is often overlooked in asynchronous communication.

The label takes a fraction of a second to type, and saves the reader(s) many seconds spent inferring what purpose the writer had.

The label makes it easy to find comments still in need of replies, and for automated security to prevent merging items with outstanding crucial comments.

The label opens the possibility of automated reporting and/or review of reviews.

Unfortunately, it’s extremely common for less senior people to interpret suggestions from more senior people as instructions to be followed even if they aren’t worded as such. Having a consistent, explicit, and unambiguous way of distinguishing between these scenarios that everybody on the team can follow easily is valuable. You can’t rely upon all developers to be good writers or even good readers, especially across language barriers.

Eh, suggestions from more senior people are in practice instructions. The risk in ignoring them is that something comes back to bite you and you look like an idiot for not following the suggestion of the senior.

Not necessarily. It depends on culture (local or otherwise). A senior needs to carefully instill an understanding of when a suggestion is optional; for some seniors, this may be "never". I personally prefer when direct instructions are mandatory but anything framed as a suggestion allows personal discretion (and requires willingness to defend having not followed the suggestion).

This is the real problem. Sometimes questions are also taken as instructions.

As a senior, you should be aware of the authority you have or that others perceive you have. Your comments will be perceived as authoritative and coming from experience.

And they might very well be! But just make sure you get a rapport with the reviewed party, and prefix or suffix the comment with a "just a suggestion" or an explicit "I'm not asking you to act on this remark". With a semantic review, you can shorten it to "nit:".

> prefix or suffix the comment with a "just a suggestion"

That's what the article suggests, using "Suggestion:" as a prefix.

> or an explicit "I'm not asking you to act on this remark"

Same, the article suggests the "Remark:" prefix. Both are more terse than what you suggest here.

I agree. For example just prepending "Question" to a sentence ending in a "?" did not convey anything more to me. Worse the Question that this OP posted in his example could have actually been constructed in a safer manner if safety was what this environment was most under threat. I did sense there was a need to robotify humans and sure enough the "Source" confirmed it (Google Software Engineering Practices)!

The question label is interesting, since I often ask questions in code review. However, the answer is only sometimes relevant in terms of just learning something. Someone else reading the code having a question is a signal that the code may not be as obvious as it should be and therefore should be changed (if only by replying to the question in the form of an added comment).

Of course sometimes the reviewer is just dumb/tired/distracted and a simple explanation is all that's needed....

Experienced coders will understand this and often respond to a question with a code change rather than an answer. And junior coders pick it up quickly with mentoring.

One use case for the question label is a bit of code that you're unfamiliar with, but which is commonly used by more experienced developers. That said, it can be a good primer for the author to update the code, add explanation, or use simpler code structures so the code becomes more obvious.

Like, "what does ??= mean? I've never seen it before" in modern JS. I've had to adjust to code like that for a bit because I've been doing JS since ES3/4 and may have missed some developments (or the general availability of those).

From my experience, the challenge of a good review lies less on the how to convey but more on the capacity to set the cursor between a suggestion and an important remarks.

The only one that makes sense to me is the "suggestion", to clearly indicate that it is OK if the code is not changed or that the change is not that important.

For the rest, isn't it obvious when an question is a question or a hint is hint? If you understand English then the meaning of each comment should be clear.

Well there are people who take every hint as a command to change it right now. Though I'm not sure if labelling it as a hint would prevent that.

It is unfortunately very common for people to make requests or demands in the form of a question.

> If you understand English

See that's where your assumptions already fall apart; keep in mind a lot of developers do not have English as their first language, let alone the cultural differences and nuances if they seem to have a good grasp of the language. "Can you change this please?" can be seen as a friendly request by one, a helpful suggestion that can be ignored by someone else, or a passive-aggressive "You Have To Change This Or Else" reply by yet another. It's not as straightforward as you wish it was or as you experience it to be.

Besides, English is not even a good international language; it's three different languages in a trenchcoat.

There's another fundamental problem we often don't talk about. It takes years for developers to learn how to accept feedback and even longer to know when to reject it.

Comparing this with designers, they go through rounds of feedback constantly in college. They come out as feedback/revision machines.

Might be off-topic but this feels like one of the better sites on my mobile device: works well, the padding/whitespace is on point and the fonts and their sizes are wonderfully readable. What a pleasant experience.

Thank you for your kind feedback. As a backend developer, I wouldn't have been able to build this site from scratch. Instead, I opted for an HTML5 UP! template (https://html5up.net/), which turned out to be one of the best 20 bucks I've ever spent.

The blog post explores the idea that code review comments should be more than just polite and respectful – they should convey their intent clearly and effectively. As developers, we're well aware of the complexity of code, and communication around it should be streamlined and efficient.

Semantic comments come with labels that convey their purpose, whether it's a simple remark, a question seeking an answer, a hint for future consideration, a suggestion open for discussion, an important point requiring change, or a crucial issue that must be addressed.

While I can see how this can work for code reviews, it ignores general communication overall. I find that if someone comes across a certain way in code reviews that you need to adopt this, it's probably an issue with communication in general, and should be addressed. This doesn't mean the person is bad, but that they are probably poor communicators. This means training. We train in other areas where we are deficient, so it's not unreasonable to expect that if communication is critical to your business that you wouldn't also train people on how to communicate well.

However, while "strong communication skills" is something we say we look for, it's not something we actually test for. What does "strong communication skills" actually mean? Does it mean you know how to use a spell checker? A grammar checker? Read and write in your native language?

Personally, I've always taken "strong communication skills" as being able to communicate with other people to achieve the goals of the company. And if one of the goals of the company is creating a team of people that enjoy working at the company, then communicating in a way that doesn't achieve that flies in the face of a goal of the company. In other words, you are lacking in "strong communication skills."

tl;dr: Does "strong communication skills" really matter or is it just a platitude?

Reply on news.ycombinator.com