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. -- Jeremy Stanley __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
