Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Semantic Code Reviews – Simple and direct comments without drama

m31coding.com

Post reply on HN
Print view View original

31–40 of 49 posts

Re: Semantic Code Reviews – Simple and direct comments without drama

#31

Post by ahoka »

JimDabell wrote: post #14
alexmolas wrote: post #3

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.

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…

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

Re: Semantic Code Reviews – Simple and direct comments without drama

#32

Post by Cthulhu_ »

jcarrano wrote: post #25

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.

> 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.

Re: Semantic Code Reviews – Simple and direct comments without drama

#33

Post by Cthulhu_ »

gampleman wrote: post #17

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 d…

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).

Re: Semantic Code Reviews – Simple and direct comments without drama

#34

Post by myth2018 »

swader999 wrote: post #11

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

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.

Re: Semantic Code Reviews – Simple and direct comments without drama

#35

Post by Cthulhu_ »

jlg23 wrote: post #8
alexmolas wrote: post #3

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.

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-defin…

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.

Re: Semantic Code Reviews – Simple and direct comments without drama

#36

Post by Cthulhu_ »

JimDabell wrote: post #14
alexmolas wrote: post #3

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.

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…

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:".

Re: Semantic Code Reviews – Simple and direct comments without drama

#37

Post by baby_souffle »

duringwork12 wrote: post #22
ajanuary wrote: post #10

Earlier quoted context omitted.

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?

Re: Semantic Code Reviews – Simple and direct comments without drama

#38

Post by flashgordon »

jameshart wrote: post #19

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, pond…

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!

Re: Semantic Code Reviews – Simple and direct comments without drama

#39

Post by flashgordon »

alexmolas wrote: post #3

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 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)!
Post reply on HN