Page MenuHomePhabricator

+2 for Jakob in mediawiki/*
Closed, ResolvedPublic

Assigned To
Authored By
Addshore
May 20 2018, 5:45 PM
Referenced Files
None
Tokens
"Like" token, awarded by WMDE-leszek."Meh!" token, awarded by JStrodt_WMDE."Y So Serious" token, awarded by Kizule."Like" token, awarded by Florian."Like" token, awarded by Vituzzu."Like" token, awarded by Tonina_Zhelyazkova_WMDE."Like" token, awarded by Ladsgroup."Meh!" token, awarded by WMDE-Fisch.

Description

I would like to nominate Jakob for +2 in mediawiki/ repositories.

Code reviews: https://gerrit.wikimedia.org/r/#/q/reviewedby:%22Jakob+%253Cjakob.warkotsch%2540wikimedia.de%253E%22
Merged patches: https://gerrit.wikimedia.org/r/#/q/owner:%22Jakob%22+status:merged

Jakob has been working with mediawiki for some years. He has worked on features and extensions such as Wikibase, WikibaseLexeme and RevisionSlider. I have no doubts.

Event Timeline

I haven't looked at any individual commits, but looking only at the logs, I note commits are only in Wikibase*, RevisionSlider, Echo and wmf-config. Notably none in mediawiki/core, nor other mediawiki repos. (Including unmerged commits and review labels.)

We have historically didn't have many access groups (SVN made that difficult), but many repositories now have dedicated access groups, including higher-level groups. And I note that Jakob has actually already been added to the Gerrit access groups for WikibaseLexeme, RevisionSlider, and ldap/wmde, which grant merge rights to these as well as other Wikidata repositories.

There is currently not much written or established socially about how or when we hand out access or what that means with regards to ownership and responsibility But, at this point I feel uncomfortable to support mediawiki rights for Jakob, because it's not clear to me what changes would be appropiate to merge. Specifically, reading the text in our +2 policy about "merging changes one is qualified to review", I do not know what changes Jakob would be qualified to merge outside the repos he already has merge rights for? (Given there is no history of contributing to or reviewing changes for any of these repositories).

Actually the code review list above seems incomplete to me, please rather see reviewedby:"Jakob <jakob.warkotsch@wikimedia.de>". Reviews outside of WMDE projects: reviewedby:"Jakob <jakob.warkotsch@wikimedia.de>" -project:mediawiki/extensions/RevisionSlider -project:mediawiki/extensions/WikibaseLexeme -project:mediawiki/extensions/Wikibase -project:mediawiki/extensions/WikibaseMediaInfo.

@Jakob_WMDE definitely knows what he's doing and I have no doubts he knows what he is qualified to review (and what not). So 👍 from me! 🙂

because it's not clear to me what changes would be appropiate to merge.

I think the key part of the +2 policy is indeed https://www.mediawiki.org/wiki/Gerrit/%2B2#+2_is_for_things_you're_qualified_to_review.
I can think of / go and find possibly hundreds of patches in mediawiki/core over the past 6 months that @Jakob_WMDE would be qualified to merge.

I note commits are only in Wikibase*, RevisionSlider, Echo and wmf-config.

All of which are deployed cluster wide, (Well, Wikibase isn't actually on Wiktionary).

Another thing I'll vaugly point out here is:

From mediawiki.org
+2 rights on MediaWiki core and extensions and skins are granted to:
Wikimedia Foundation engineers who've passed internal hiring standards (which includes review of previous development work, task assignments, and more), at the discretion of hiring managers; and

All WMDE engineers also pass internal hiring standards. I also have no doubt that @Jakob_WMDE's manager would approve of a +2 right for core. (not that that actually matters as it is not written in the policy on mw.org)
Another thing to note is that not all work for WMDE engineers takes place on Gerrit currently, so a look at the gerrit links in the description may not be representative.

From mediawiki.org
This is a big deal. Your merge could cause Wikipedia or other sites to fail. It could create a security vulnerability that allows attackers to delete or corrupt data, or to gain access to private information. And in the more common case, it could cause technical debt to increase if the code doesn't have tests, is poorly implemented or poorly socialized.

Jakob already has the technical and social ability to do all of this, but he has not in 4 years.

For me +2 is about trust, I trust Jakob to only act where he feels changes would be appropriate to merge.

Actually the code review list above seems incomplete to me, please rather see reviewedby:"Jakob <jakob.warkotsch@wikimedia.de>". Reviews outside of WMDE projects: reviewedby:"Jakob <jakob.warkotsch@wikimedia.de>" -project:mediawiki/extensions/RevisionSlider -project:mediawiki/extensions/WikibaseLexeme -project:mediawiki/extensions/Wikibase -project:mediawiki/extensions/WikibaseMediaInfo.

@Jakob_WMDE definitely knows what he's doing and I have no doubts he knows what he is qualified to review (and what not). So 👍 from me! 🙂

Thanks for the link fix, also fixed in the description.

These requests are normally open for roughly 1 week and this request has now been open for over 2.

To avoid this sitting stale would anyone else like to chip in or judge consensus(not really much discussion to go on)?

Sorry, slipped out of my queue. I'll poke @Krinkle to ask him to respond, but either way I think we can close this by Wednesday.

Actually the code review list above seems incomplete to me, please rather see reviewedby:"Jakob <jakob.warkotsch@wikimedia.de>". [..]

Thanks for the link fix, also fixed in the description.

For the record, I did write my comment based on all his activity in Gerrit, not just the links in the task description.

@Legoktm Yes, consider my questions addressed.

Legoktm claimed this task.

Done. Congrats Jakob :)

Vvjjkkii renamed this task from +2 for Jakob in mediawiki/* to jkcaaaaaaa.Jul 1 2018, 1:08 AM
Vvjjkkii reopened this task as Open.
Vvjjkkii removed Legoktm as the assignee of this task.
Vvjjkkii triaged this task as High priority.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed a subscriber: Aklapper.
CommunityTechBot renamed this task from jkcaaaaaaa to +2 for Jakob in mediawiki/*.Jul 1 2018, 4:36 PM
CommunityTechBot closed this task as Resolved.
CommunityTechBot assigned this task to Legoktm.
CommunityTechBot raised the priority of this task from High to Needs Triage.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added a subscriber: Aklapper.