4 ms·

I'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 (

by tomp 7y ago

I'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?

H8crilA 7y ago
Don't the tools you use allow for selecting whether a comment is blocking or not? This is like the #3 feature of code review software. #2 would be the "approve" button, and #1 would be showing the diff.
- joombaga 7y ago
  GitHub doesn't.
kubanczyk 7y ago
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.
- tomp 7y ago
  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 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.
  - andr3 7y ago
    I think "passive-aggressive" is a bit too strong. If you leave a comment, even if not blocking, resolving that discussing can simply be a signal that the author of the MR has acknowledged it. 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.

by tomp 7y ago

H8crilA 7y ago

Don't the tools you use allow for selecting whether a comment is blocking or not? This is like the #3 feature of code review software. #2 would be the "approve" button, and #1 would be showing the diff.

joombaga 7y ago
GitHub doesn't.

kubanczyk 7y ago

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.

tomp 7y ago
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 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.
- andr3 7y ago
  I think "passive-aggressive" is a bit too strong. If you leave a comment, even if not blocking, resolving that discussing can simply be a signal that the author of the MR has acknowledged it. 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.