1. 22
  1.  

  2. 11

    These aren’t for comments, but rather for replies on a review thread. I think it is unwise to overload the term ‘comment’ in computing.

    For code comments, I have been using https://www.python.org/dev/peps/pep-0350/ for this for a long time, and recommend it to others.

    For review responses, I suppose this looks decent enough, although the use of bold assumes styled text; I would prefer all-caps, as has been conventional in unstyled text for quite awhile. When styles are available, bold and all-caps is quite visually distinct.

    1. 4

      These aren’t for comments, but rather for replies on a review thread. I think it is unwise to overload the term ‘comment’ in computing.

      These are comments, readers are supposed to understand that we’re talking about something different from code comments by context. This is absolutely not an unreasonable expectation. Both my kids have understood contextual words without being taught. Context really is intuitive to human nature and it’s perfectly reasonable to use the same word in different contexts to mean something different.

      1. 4

        “Reviews” is the standard word for this.

        1. 1

          The only real context here outside of TFA itself is computing, or maybe slightly more broadly, technology. All we see on this site, which is generally full of programming minutia, is “comments”, in both the title and domain name. The use of the word “conventional” only makes it worse: conventions in code comments are an almost universally recognized and common thing, conventions in reviews, not so much. One might even argue that this is nearing territory considered off-topic on this site (being not particularly technical).

          I’d be low-key surprised if anyone here assumed differently. This is actually my second click through to this article, because although I read the whole thing the first time, it didn’t even occur to me that this link was to that article, and not something on code commenting practices or whatever that I missed before.

          Sure, anyone who actually reads the whole thing and comes away confused… well, has bigger problems… but it’s still a poor choice of words. Maybe this is a superficial bikeshed, but that sort of thing is pretty important when the whole point is to define a soft standard for things with a standard name. Even in the context this is specifically intended for (code review), I’d assume that “conventional comments” was something about the code (did I get the Doxygen tags wrong or something?), because of course I would. That’s what a code review is.

      2. 3

        Praise: Who knew that HK-47 would have something valuable to teach us?

        1. 2

          Interesting idea; not 100% sure I’m convinced yet, but certainly food for thought; maybe it’s just so new that it needs to sink in. That said, one thing I’m quite surprised with, is that the author suggests for a “nitpick:” prefix to be ”(…) necessary” — whereas notably the Google Code Review Guide suggests to use “Nit:” to prefix a comment that ”(…) isn’t mandatory”. Personally, I tend quite often to prefix review comments with a somewhat long [optional][nit][style]; not quite sure how I’d be expected to mark it with OP’s proposed approach — nitpick(non-blocking):? Hm; with further thought, if I were to use this, I’d be tempted to bikeshed a bit over shortening the “decorations” from non-blocking/blocking to opt/req — thus e.g.: nitpick(opt): and nitpick(req):; also tempted to bikeshed the (if-minor) to (if-easy).

          1. 2

            Our team has a convention that PR review comments that start with ~ are nitpicks, and thus optional to fix. Add more (e.g ~~~) to signify really nitpicky comments :-)

            1. 2

              My first impression was that I don’t like this. I’ve seen plenty of the miscommunication that they’re warning about, but why not solve it with good, old-fashioned sentences? If you want to say that something is not essential, you can say it casually, like you would in speech. It doesn’t have to be in a machine-friendly format. You’re not talking to a machine.

              After some thought, I’m a bit intrigued. It seems like the selling point is that the label acts as a prompt for the reviewer. In the first example, the real solution, as shown, was making the feedback actionable, not adding a label. But if labels remind people that their suggestion actually has to suggest something, that would be good.

              Still, it feels like a technical solution to a non-technical problem. If people don’t recognize that what they’re saying isn’t actionable, I’m not convinced that syntax rules will help them. They might need someone to clearly communicate to them that there’s a problem.

              Labels feel like declarations of a fact, which may be fine for “suggestion” but not for something like “non-blocking” or “optional”. Those are debatable. You should say why we should deal with it, or why we don’t have to. That encourages thinking about and discussing the idea. The author can contribute to that. Maybe they know something about the problem the reviewer doesn’t. Maybe neither person can decide whether the change is essential, and someone else should be asked. If I’m suggesting something and the only thing I say about the importance of it is “it doesn’t matter if we do this or not”, why waste people’s time and attention with the suggestion in the first place?

              1. 1

                We have been doing something similar, but with less levels.

                • nit, q, and req. works pretty well for us, not that we have any parsing or anything but I agree with op that framing the comments dramatically reduces misinterpretation