+1 about Dan's comments.
1. We should not discourage all cases for -1 for questions. Because it often leads more discussion about the code issue, it is helpful for such case. Of course, it is diffcult to find a balance point, what can -1, what can 0. I don't think 0 in gerrit works well, because sometimes authors not care about that. 2. Also, for typos in comments and message, -1 makes sense too. Because 0 in gerrit means no need to improve the message and everything is good. It can lead many bad cases for cores, and non-cores. It means it doesn't matter if spell right or wrong. Best Wishes, -------------------------------------------------------------------------------- Follow your heart. You are miracle! From: Dan Smith <[email protected]> To: "OpenStack Development Mailing List (not for usage questions)" <[email protected]> Date: 04/24/2015 10:48 PM Subject: Re: [openstack-dev] Please stop reviewing code while asking questions > In defense of those of us asking questions, I'll just point out > that as a core reviewer I need to be sure I understand the intent > and wide-ranging ramifications of patches as I review them. Especially > in the Oslo code, what appears to be a small local change can have > unintended consequences when the library gets out into the applications. > > 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. > > Most of the time the author has thought about the issues and worked > out a reason they are not a problem, but they haven't explained > that anywhere. On the other hand, it is frequently the case that > someone *hasn't* understood why a change might be bad and the > question ends up leading to more research and discussion. Right, and -1 makes the comment much more visible to both other cores and the reviewer. Questions which rightly point out something which would lead to what the OP considers a legit -1 can *easily* get missed in the wash of review comments on a bug. If you leave a -1 for a question and never come back to drop it when the answer is provided, then that's bad and you should stop doing that. However, I'm really concerned about the suggestion to not -1 for questions in general because of the visibility we lose. I also worry that more non-core people will feel even less likely to -1 a patch for something they feel is just their failing to understand, when in fact it's valuable feedback that the code is obscure. As a core, I don't exclude all reviews with a -1, and doing so is pretty dangerous behavior, IMHO. I'm not sure if the concern of -1s for questions is over dropping the review out of the hitlist for cores, or if it's about hurting the feelings of the submitter. I'm not in favor of discouraging -1s for either problem. --Dan __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
