Open Stack

Excerpts from Jeremy Stanley's message of 2015-04-24 15:15:18 +0000:
> On 2015-04-24 10:23:35 -0400 (-0400), Doug Hellmann wrote:
> [...]
> > I will often ask questions like, "what is going to happen in X
> > situation if we change this default" or "how does this change in
> > behavior affect the case where Y happens, which isn't well tested
> > in our unit tests." If those details aren't made clear by the commit
> > message and comments in the code, I consider that a good reason to
> > include a -1 with a request for the author to provide more detail.
> > Often these are cases I'm not intimately familiar with, so I ask a
> > question rather than saying outright that I think something is
> > broken because I expect to learn from the answer but I still have
> > doubts that I want to indicate with the -1.
> [...]
> 
> Thinking about this though, for the benefit of non-fluent readers
> and cultures where rhetorical questions are a less common construct,
> it's probably better if we make a conscious effort to actually say
> what we mean and give directly actionable feedback when reviewing.
> 
> "Please add code comments here explaining the behavior in the case
> where Y happens, since it isn't well tested in our unit tests." (Or
> even better, "please add tests!")
> 
> A lot of the good "questions" I see in reviews where there's a -1
> (and I'm frequently guilty of this too) could be much more
> effectively phrased as requests to improve code comments, use
> clearer syntax, add more detail in commit messages, or even better
> test the code being added/changed.

That's a fair point. I tend to think of the questions as a way to start
the discussion about the patch, rather than a subtle hint at what I
think might be wrong. If that conversation reaches a point where more
work is obviously needed, then I go ahead and make that request more
plainly.

Doug