I appreciate that Paladox has done a lot of great work. He's also demonstrated much growth over the years.

My main concern is that the vast majority of his patches are small maintenance patches. To be clear, this is important work. What I am about to say should not be seen as anything negative about Paladox. However, I feel that the main role of a code reviewer is to analyze the code for unexpected complications or side effects. I think the key skills involved in determining that are developed when writing more complex features or refactorings that change the ways components work in non-trivial ways. I worry that paladox does not have enough experience with that sort of development to be able to catch unexpected interactions with code under review.

I think the total quantity of commits for this candidate is rather misleading. Instead I took a look through his merged commits (in master branch) over the last 2 years:

• In mediawiki core - There are 8 commits in this time period (technically 7 but one was right at the cut off). Over all time, there has been 100 commits.
  • 2 commits related to small features (ipv6 in memcached 1 2). 1 commit re-adding a fixed version of patch that was previously reverted (T352444). 5 commits fixing php or deprecation warnings (generally 1 line fixes).
• non-mediawiki core (All other repos, including both WMF deployed and non-wmf): 41 commits
  • There is an example of a large complex change here: This change to how SocialProfile manages files
  • Small features/bug fixes - ipv6 in poolcounter, ipv6 in job runner, Download link in TMH
  • The rest are fixing of deprecations, php, or phan warnings (mostly 1 line fixes).

So in conclusion, in the last 2 years, there has basically been only a single patch of his merged that was remotely complex and only about 5 that are more complex than a one-liner. I would normally expect someone with +2 on MediaWiki to demonstrate more complex patches than that. Additionally, I wouldn't exactly consider the amount of activity in the last 2 years to be super high.

Again I want to really emphasize that I do believe paladox does great work. This comment should not be taken as saying anything negative about Paladox or his work. All I'm saying is I would generally like to see a different type of work in candidates for +2.

Edit: I would also add, that I am a little concerned about the level of testing this contributor does with his patches. There are only a small number of non-trivial patches to look through, so its hard to get a sense of this. However looking through the comments on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SocialProfile/+/444441 is a bit concerning, and similarly https://gerrit.wikimedia.org/r/c/mediawiki/core/+/972724 suggests a lack of testing. Normally I would not care about just 2 examples, everyone occasionally misses stuff when submitting code. However given how few of his patches are more than one line fixes, I do find it a bit concerning that two of them showed signs of insufficient testing.

In T355619#9479498, @Bawolff wrote:

I think the key skills involved in determining that are developed when writing more complex features or refactorings that change the ways components work in non-trivial ways. I worry that paladox does not have enough experience with that sort of development to be able to catch unexpected interactions with code under review.

My main expectation is that someone with +2 rights will have to good sense to not approce things they don't sufficiently understand. There are certain kinds of changes that I would not give a +2 on, e.g. config changes and changes to CI setup. Complex code changes are not the only kind of change that need review.

I trust Paladox to apply good judgement when deciding what to use their new +2 rights for.

In T355619#9479729, @daniel wrote:

In T355619#9479498, @Bawolff wrote:

I think the key skills involved in determining that are developed when writing more complex features or refactorings that change the ways components work in non-trivial ways. I worry that paladox does not have enough experience with that sort of development to be able to catch unexpected interactions with code under review.

My main expectation is that someone with +2 rights will have to good sense to not approce things they don't sufficiently understand. There are certain kinds of changes that I would not give a +2 on, e.g. config changes and changes to CI setup. Complex code changes are not the only kind of change that need review.

To clarify, my concern is not that paladox lacks experience with complex changes and therefore wouldn't be able to review them. I do not believe a +2'er has to be able to review everything.

However, I believe even for the simplest one line change, a reviewer would be expected to verify that the change fits into the logic of the surounding code. That's the types of changes i am concerned are missing here - changes with multiple statements (even just 2) that logically depend on each other to demonstrate function-level reasoning. I am not saying that a candidate for +2 needs to have large refactorings under their belt.

@Paladox is my de facto point of contact for patches I write for Gerrit. He is quite speedy, productive and has valuable reviews.

Oppose, largely per Bawolff. MediaWiki /is/ a complex codebase and I do not think one can effectively review even small patches without a good ability to understand how a piece of code fits into the larger picture. I am just not seeing that demonstrated here. Sorry.

In T355619#9479984, @hashar wrote:

@Paladox is my de facto point of contact for patches I write for Gerrit. He is quite speedy, productive and has valuable reviews.

This has not much to do with having the right to merge patches in MediaWiki, though.

I concur with the sentiment from taavi and Bawolff.

I'm not the most active Mediawiki+extensions contributor so that's only small two cents from me. I'm quite impressed @Paladox kept to hang around for that long after T106359 had happened, which has been quite a disappointing read.
The number of contributions is quite impressive. Those might be small contributions but I think they deserve opportunity to demonstrate the disputed understanding of the codebase in reviews. I think granting +2 rights could be a nice encouragement and motivation to do more of these (that's how I understand to above comments, that there's not been enough of complex mediawiki changes reviewed abd submitted by Paladox)

+2 access to MediaWiki core and the hundereds of related extensions and skins not a participation trophy we give from being around for many years. It is, to quote the policy, a "strong expression of trust, and trust is maintained through good judgement and careful action".

I do not see how T106359, indeed from 2015, is any relevant in determining whether that trust exists today. Looking at the list of their recent Gerrit changes and excluding backports, I am mostly seeing one-line fixes for PHP deprecation warnings with no Phabricator tasks associated. That, combined with the unusually large portition of abandoned patches, does not to me demonstrate the the knowledge of the various quirks and best practices involved with writing modern, well-maintainable MediaWiki code which I consider a core requirement for having +2 access to mediawiki/*.

While I agree with a lot of what was said above I still wonder what the concern is? What are we afraid of? That people with +2 rights start running around merging random patches that should not be merged? That people with +2 rights make mistakes?

If that's the only thing we are afraid of then @Paladox does have my full support. They are high up on my mental list of long-term contributors. Their actions and comments on Gerrit are slow, steady, and calm. I occasionally add them as a reviewer to my patches and try to help reviewing their patches in a timely manner whenever I can. Their patches might be small (or are they?), but so are mine.

Why not just ask @Paladox if they even want the +2 button?

What are we afraid of? That people with +2 rights start running around merging random patches that should not be merged? That people with +2 rights make mistakes?

This seems more like an argument that we should abolish this process and give all contributors +2 rights. Perhaps a reasonable argument to make but that would certainly be quite a shift

In T355619#9481817, @thiemowmde wrote:

Why not just ask @Paladox if they even want the +2 button?

I did ask, by private email. Although in hindsight, maybe I should have explained the process and the risks of receiving negative comments.

I also want to note that I used male pronouns in the task description according to his preferences set on mediawiki.org, which incidentally concurs with the given name in his email address.

This seems more like an argument that we should abolish this process and give all contributors +2 rights.

That's not remotely what I said. Not "all contributors" are active for more than 10 years.

In T355619#9483624, @thiemowmde wrote:

This seems more like an argument that we should abolish this process and give all contributors +2 rights.

That's not remotely what I said. Not "all contributors" are active for more than 10 years.

If your argument is that we should grant +2 simply because not much can go wrong, doesn't that apply equally to someone you have never met as to someone whose been here a while? If we are not taking into consideration the things this person has done over the last 10 years, then why is it a selling point that they have been doing them for 10 years?

To put it bluntly, while I do believe negative things that happened years ago should be considered water under the bridge, i think that goes both ways - negative things from years ago is not positive evidence either.

Again, that's not what I said. Nobody here is talking about "someone we have never met".

Reviews by Paladox: +2, +1, -1, -2, comments.

I'm on the fence about this. On one hand, he hasn't really done any code review (last time he gave -1 to a change made by someone else was 4 years ago). I think we generally expect a track record of good -1 reviews and comments before giving merge rights.

On the other hand, his current use of +2 (on non-Wikimedia-production repos where he has it) seems positive. (There is no way to search for reverted changesets in Gerrit, but I checked his last ~10 merges, and none of them were reverted. See also T158019#3027351, if you can.) He is also active at Gerrit (the opensource project): patches, reviews; that includes nontrivial patches and also apparently has merge rights. (Gerrit doesn't run close to master like Wikimedia does so merges are less risky, but it's still a quite large project.) If someone who is trusted with merge rights at another large opensource projects, it should probably be fine to trust them here as well?

I worked with Paladox in the past on the gerrit codebase, I trust him to use his +2 rights wisely.

I'm getting English Wikipedia RfA vibes here :(
You don't need to be a content writing machine to be a good admin, and similarly I don't think you need a ton of XL patches to prove yourself as an engineer capable of reviewing other's patches.

The Gerrit policy was quoted above, that "+2 is a strong expression of trust, and trust is maintained through good judgement and careful action". To me, Paladox ticks all those boxes.

All engineers hired by WMF get +2 rights. This happens regardless of their prior experience with MediaWiki, which sometimes is next to nothing. This is because as an employee, there's an inherit trust that they will only merge patches that they understand, have properly tested, etc. I feel like 10+ years of contributing to MediaWiki surely lends the same level of trust, if not considerably more… If this request fails, maybe we should just hire Paladox instead? :)

maybe we should just hire Paladox instead?

+1 that would work for me as well :-]

I already raised support on a list. For here I would just like to say that if a +2 is rejected based on an argument that most patches are small maintenance patches but at the same time people involved in more complex changes are not willing to review and merge small maintenance patches (because they are considered trivial) then that is kind of a lose-lose situation.

It would be even more important to fix the general situation where it's hard to get reviews, especially without having to ping in real time because people filter their Gerrit mail.

In T355619#9510445, @Dzahn wrote:

but at the same time people involved in more complex changes are not willing to review and merge small maintenance patches (because they are considered trivial)

In my experience getting people to merge working tiny patches has never been an issue.

I'd like to start off with a thank you for the nomination! To get into things I'd like to bring up my +2 rights on Gerrit-review that @Tgr mentions above. Whilst yes they aren't bleeding edge in the backend they are more so for the frontend. Users are more easily impacted if there was malicious code inside the frontend code. Not to mention people building or running it locally. A maintainer breaching trust would be huge and also have reputational damage especially as big companies use it such as Wikimedia and Volvo and more. I wouldn't go around merging anything I don't understand. (I show this on Gerrit-review).

I have submitted feature changes in the past (to name a few but also with help): https://gerrit.wikimedia.org/r/c/mediawiki/core/+/158505 and https://gerrit.wikimedia.org/r/c/mediawiki/core/+/193434. I have progressed over the years by learning. I can't promise to never make a mistake as I'm only human but I can offer reassurances that I'd be asking other reviewers if I don't understand something. I'd like to work on bug fixing that a lot of my changes already focus on and also review https://gerrit.wikimedia.org/r/c/mediawiki/libs/Shellbox/+/982498.