> * Should we extract this to a separate function? Reads very passive aggressive. No. > * Could you extract this to a separate function? Yes, I could. > * I

> * Should we extract this to a separate function? Reads very passive aggressive. No. > * Could you extract this to a separate function? Yes, I could. > * I would extract this to a separate function. Good for you. > * This could be extracted to a separate function. This one's OK as a style suggestion that you won't block the merge over. > * This should be extracted to a separate function. This is kinda, sorta OK, if you're going to block the merge if this isn't fixed. > * Extract this to a separate function. This is better because the tone matches the intent: it's a command. This gets addressed, or no merge. "This should..." is indirect—you're not stating what you actually mean. This phrasing is better. 4 is the best of these for optional suggestions, 6 is best if you consider the fix a requirement for a merge. In general, say what you actually mean—dancing around it leaves room for miscommunication and can give all kinds of bad impressions. Of course, that can also mean weakening language: don't command a change that you don't consider a big enough deal to block the merge. [EDIT] And, as others have noted in the thread, of course expanding with justifications is always a good idea. "Do this" without a "why" should be avoided. But lead with what you want done, not the why—that follows.