The site request documentation currently says this:
Another pair of eyes will review this patch in Gerrit.
Although in reality that rarely happens, and instead most straightforward configuration patches are reviewed by the deployer during the window. I think that's fine as I would expect deployers running the windows to be familiar with the config repo and able to review the changes. It'd be good to update the documentation on what we expect to happen in reality instead.
Counterpoint: knowing the config settings doesn't mean understanding the code activated by those changes or its possible impacts. At least, not for me. Some areas I know, and some I don't.
In any case, clarification would be good.
I have another take on this. I think, given those are config patches, the main question is not the how, but the what.
In some cases, a configuration is needed for operational/emergency reasons, and the deployer knows it's ok, I think it is ok if they're the only reviewer.
But in case of a config patch that changes the behaviour of a wiki we should have:
Establishing a culture of getting a +1 for safety / QA makes sense to me, even if that would result in waiting additional hours or days before deploying some features. For WMF staff, ideally the +1 comes from a teammate. In other cases, it would be good to get a +1 in advance of the deployment window from others engaged in that particular part of code. As a last resort, people around during the deployment window could give a +1.
I'd support removing this point from the documentation.
I always assumed that the deployer serves as the "other pair of eyes" and reviews the patch before deploying, and I've seen all of the deployers do that. I think we should only ask users to find another reviewer if the deployer is not comfortable with being the reviewer as well (and we already document at https://wikitech.wikimedia.org/wiki/Deployments that "Your patch may or may not be deployed at the sole discretion of the deployer", so deployers should feel empowered to request that). There are definitely some changes that you can't reasonably review in 5 minutes before deploying them, but it's a minority.
My main concern here is for the patches often proposed by community members to change the configuration of their wiki. It already takes enough effort to follow this process, and I wouldn't want to make it more difficult for people to control their own experience on the wikis.
It might still be a good habit for WMF staffers to get into to do a code review on config patches, but I think this is a different process from the site requests that we're discussing here.
I'll grant that most of the time a deployer is the only reviewer. But having more reviewers makes deploying easier so I'm pro leaving the language.
Backport windows are short—you've got a minute or two to review each change. Any signal that makes this short review window easier is helpful (speaking as a deployer).
I'll also conceded that most patches in backport windows don't have +1s. So maybe the language to change is the misleading adverb "generally"; i.e., "A request generally follows this workflow" → "A request ideally follows this workflow"
+ Wikimedia-Site-requests which is the tag used to update configuration. It has several watchers which I guess would be interested in commenting on this topic.
The experience of getting things done as a volunteer (in my case, getting a new extension deployed) is already incredibly difficult if you "don't know the right people", so I'd not support anything that would add hurdles to that experience.
At minimum there should be clear documentation of _who_ the +1 should be sought from. Ideally a list of reviewers should be provided. Don't just assume a volunteer knows the names of people who can get a config change reviewed.
Another pair of eyes will review this patch in Gerrit.
I always assumed deployer would be reviewer of patch, also it would be fair if deployer would ask the patch to be reviewed by others if they are not comfortable deploying config changes themselves. As matmarex said it would be inconvenience to new patch writer to seek review for small configuration changes.
+1
I boldly edited the policy to try to capture the following:
Edit is here: https://wikitech.wikimedia.org/w/index.php?title=Wikimedia_site_requests&diff=prev&oldid=2121061
Does that capture reality better? And does that cover the concerns raised here?