Skip to content

Update behavior to not remove yourselves from the reviewers list

Nick Veenhof requested to merge nick_vh-master-patch-82959 into master

What does this MR do and why?

Describe in detail what your merge request does and why.

!56802 (merged) explained why this process was introduced. From what I can understand, and @john.mcdonnell also has that hunch, it used to be unclear in the sidebar who already reviewed something and who hasn't. (https://docs.google.com/presentation/d/1HogTpvGwIkWnPZx2nOprY6ktUj5BMEXme60Csurv-Y0/edit#slide=id.gc3b63bfc30_0_57 Slide 12)

Today, it is clear in the sidebar with a if you approved the MR already or not. Unless I'm missing something, I don't see any added value in removing yourselves. Even more so, I experience the downsides if other reviewers remove me. This doesn't allow me to keep a tab on the MR to understand the future state & or future feedback it gets from other reviewers. I would love to learn from other reviewers and this flow doesn't fully allow that.

Another argument to keep myself in the reviewer list is that my pre-frontal cortex has remembered that this MR exists in the "Review requests assigned to you" list. That same pre-frontal cortex is very confused if it is suddenly no longer in that list and I need to remember other ways of finding the same MR to do a follow-up.

@m_gill , you might be best suited to give us a deeper insight in whether this change makes sense or not. Maybe I'm missing a roadmap epic / issue and you/we were already working towards this?

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Nick Veenhof

Merge request reports