On 2020-02-13 11:55, Vlad Zahorodnii wrote:
On 2/13/20 8:11 PM, David Edmundson wrote:
I'm also seeing a rising amount of pushing without review on the core
repos. I would like for us all to (nicely) call that out if we see
any instances. Reviews are super important, the best time to fix a bug
is before it even happens. Even for small commits and "safe" commits.
Yes, ideally each commit must be reviewed by somebody. But what if one
doesn't get _any_ feedback from code reviewers for weeks or even worse
for months? What one should in that case? Speaking for myself, I manage
such cases by sending private messages asking to do a code review and I
feel very bad after doing this because I know people whom I talk to are
are busy with their own stuff and they don't really want to deal with
"problems."
I think we first need to understand why people are pushing without any
code review. Is it just because of desperation? or is it just because of
not caring?
Personally, I only ever commit without review to fix very small UI
glitches or correct bad strings, and even then only when I'm 100%
certain that the commit is good because the same thing was approved
elsewhere or it is self-evidently, uncontroversially correct. For
example,
https://phabricator.kde.org/R120:2a0b1f570b3a15f8c12267889e46d9dabacb4b6d
and
https://phabricator.kde.org/R236:2157e258721be0c81477b2e305e6d697cafb597d.
The flip side of asking for everything to go through review is that
reviewers need to be on top of their review queues, or else even trivial
changes get frustratingly delayed for days or weeks for no real gain.
But in general, I agree with the sentiment that everything should go
through review, and especially anything anything that's remotely
complicated or that's used by downstream code.
Nate
