← Back to context

Comment by clates

4 hours ago

Our team tends to prefix all our comments with one of

* thought: Maybe foo'ing is more common in the future - we can refactor if that happens.

* change: This is a leaky abstraction, would prefer to see this modeled like bar instead.

* nit: Naming seems a little unintuitive, consider "Baz", "Boo" maybe?

* fix: This unit test is validating the wrong field.

* chat: This is a big decision and would dictate how solutions of this category look like going forward. Let's bring this to the team first.

----

With the idea that some of those prefixes are stopping the PR until they are changed, and some are just a "take it or leave it" type comments. It makes it unambiguous to the opener that you consider these X things as "We've gotta get on the same page" and these Y things as "Stated preferences" or "just an observation".

word of warning - don't feel bad if you leave a nit, the other person disagrees and ignores it. If you felt strongly about it, it shouldn't have been a nit.