Right now the policy only requires a wikitech-l email for requests for mediawiki/* +2 rights. I think we should expand that requirement to all Wikimedia-deployed MediaWiki code, as that access is sensitive enough so that requests should be seen by the wider developer community and not just the people who happen to follow the Gerrit privilege request tag. An email to wikitech-l has been sent for the past couple of such requests and I think it's worked fine.
Talk:Gerrit/Privilege policy
Should this be changed to the new body?
If we had a new technical standards setting/reviewing body to change it to that would be excellent.
That group has no met for about a year now... Technical decision making/Technical Decision-Making Process Retrospective and Consultation plan
Hello,
I have since retired from development so this does not affect me much, if any, nowadays. But in the past, each time I was in need to request something to the gerrit administrators (mostly settings, or when clerking on MediaWiki +2 granting/removal Tasks) there was no easy way to reach the Gerrit administrators as a group; so I had to send some emails here and there until someone replied; which made me feel rather bad as I felt I was always putting the burden on the same couple of people that were responsive.
Given that now -and since some time- Gerrit adminship is managed through an LDAP group, I was wondering if we could create an email alias (EXIM? Wikimedia Google group?) such as "gerritadmin[at]wikimedia[dot]org" and subscribe all Gerrit Administrators there (except the bot I guess) so in case anyone needs an admin there's an easy way to reach 'em all through a single email to a single place.
I think that a gerritadmin[at]wikimedia[dot]org address exist or existed per phab:T54150 and got deleted per phab:T220843. If I remember rightly, I discussed the idea briefly with Hashar or Tyler a while ago in the RelEng channel. I can't remember. The later task also mentions a gerrit-admin[at]... address too.
Thanks.
Can we continue to self-merge this kind of changes? Given that mediawiki/extensions.git changes mostly involve adding or removing submodules to track newly added or archived extensions, can I consider this, in light of this new policy, that this qualifies as trivial to self-merge? (exception for the python and shell files which do indeed require review and I wouldn't self-merge). It looks (and thus why I've been doing that) that most patches are self-merged there when it's just adding or removing a submodule as it's purely routine. Thanks.
Edit: other cases I need to self-merge is to when I am creating a new Gerrit repo or adjusting its permissions.
If you're doing it already and nobody cares, it's probably fine. The section on self merging is merely descriptive of the current situation. It has plenty of wiggle room to allow things that are happening already.
I also want to emphasize that we're not going to suddenly revoke +2 access of a valued contributor for violating some narrowly interpreted clause in the policy. I can't speak for all situations or all committee members, but if someone complains that you shouldn't be doing a particular variety of self merges, the obvious outcome of that is to ask you to stop doing it.
The policy defines an escalation path for complaints which allows them to be handled with common sense and without needless public humiliation.
Cannot find that term on this page. File a ticket under https://phabricator.wikimedia.org/tag/gerrit-privilege-requests/ ? Some other way? Brought up in PS6 on https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/574865/.
Hello. I opened a discussion on Phabricator regarding this subject on January 2020. You can see it at https://phabricator.wikimedia.org/T238651. Regards.
It seems common sense that the section would only apply to code in Wikimedia production, just like the +2 policy, but the text doesn't actually say so. Could that be clarified? We don't want to force people to go through Phabricator privilege requests for repos under labs/tools (Toolforge), for example.
On Gerrit/Privilege_policy#Requesting_Gerrit_privileges it says that "To request membership in another group, create a new task under the Gerrit-Privilege-Requests project in Phabricator", "If there is a consensus of trusted developers on the Phabricator task, any of the Gerrit administrators can resolve the request" and "Gerrit administrators should not grant repository ownership to ordinary developers". Labs/Cloud is not part of Gerrit/Privilege_policy#Expedited_process_for_trusted_organisations and in light of Topic:Uswroi0fiud9on8o as well, it looks to me that (common sense or lack thereof) all access requests shall go through a Phab task.
Who can amend other's changes? How to deal with the message above the best way? « Saper // talk » 09:35, 12 November 2019 (UTC)
Hi @Saper,
changes (in this case patches) which are not your, can't be amended by you or me. Only by original uploader or user which have +2 in related repository.
Best regards,
Zoran.
@Saper Please try again. I made something.
@MarcoAurelio thanks, what has changed? The original change got merged, but I tried this on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/118981 (not sure how it works, since previously I have participated in this).
@Saper What Zoran mentioned above is true. Due to abusive use in the past of the ability to add patch sets to the work made by others, this ability is now restricted to patch owners, repository owners and members of some gerrit groups. This includes a group called Trusted-Contributors. I added you yesterday to test if my lecture of the gerrit permissions was right. I have removed you for now from said group because with this new Gerrit Privilege Policy in place it looks each and every Gerrit permission must be discussed and approved in advance. I have filed phab:T238649 requesting that you be formally added. Sorry for the bureaucracy, but it looks those are the rules now. I hope it won't take long to have you formally approved. Best regards.
Thanks... the policy says nothing about "trusted users" so I was a bit baffled if this is a glitch or intentional thing. Thank you again - will follow on Phab!
The policy is not quite clear for me in what a "privilege" is or how that should be interpreted. I opted to follow the safests of the paths. If the discussion results in that there's no need for Tasks to add people to that group, I'll be more than happy to follow that consensus. Regards.
@MarcoAurelio I appreciate your approach and the efforts. It worries me that this has become so burdensome. So much for "attracting new developers to the project" etc. etc. Although I am a pretty old one :)
The policy currently refers to "Gerrit Administrators" as being responsible or empowered to perform certain actions by this policy. It would however be more useful to associate these powers and responsibilities with gerrit permissions rather than a specific group, allowing the granularity of groups and permissions on gerrit to be improved as discussed on phab:T219012#5057232.
I propose to replace all references to "Gerrit Administrators" with "people with the necessary permissions on gerrit", (or an equivalent but less clunky phrase).
On one side there is the administration of the Gerrit software itself. That is the Administrators built-in group in Gerrit which is https://gerrit.wikimedia.org/r/#/admin/groups/1,members This is the equivalent of having read/write access everywhere (disk or database).
On the other hand, the Gerrit Managers group https://gerrit.wikimedia.org/r/#/admin/groups/119,members is for administration of policy / access lists / repositories etc. It is less privileged than the administrators of the software itself, but is still a very large set of permissions. The group was originally named Project and Group Managers but got renamed to Gerrit Managers in Feb 2018 by https://gerrit.wikimedia.org/r/plugins/gitiles/All-Projects/+/10107cf5056eb6ef3903f49d59cd27387831c5b5%5E%21/#F0 It is semantically broader and might be confusing :D
I think changing it to "Gerrit Managers" would be the most reasonable change.
> I think changing it to "Gerrit Managers" would be the most reasonable change.
It sounds reasonable, but in T219012 there is talk of creating a mediawiki-administrators. And further groups may be created in the future, for other repos, or with different rights.
This is why I want to avoid any group names in the policy. The policy should apply to whoever has the respective rights.
Then call it "the 'Gerrit Managers' group of individuals, or whatever it's called this week". Let them bike-shed in peace.
The link that is currently on the page links to https://gerrit.wikimedia.org/r/#/admin/groups/1,members which is not very helpful. Should it be removed or changed to something else?
I believe the current group is https://gerrit.wikimedia.org/r/admin/groups/119,members.
Gerrit Managers can change group membership only for groups they own, which is only 201 groups out of 1630. I volunteered to update the policy to say that Gerrit Managers can change groups, but I'm not going to do that when it's not true. 201 is suspiciously close to a round number, is it possible that someone tried to edit all the groups to be owned by Gerrit Managers but it failed due to a batch size limit? If I do the following query on the Gerrit database:
select group_id,name,owner_group_uuid='93b1e277b72d0e0a883afbc0a87948dd6dd0d7b7' as managed from account_groups where name like 'extension-%';
I get 1022 such extension-specific groups, of which 118 are owned by Gerrit Manager. The groups owned by Gerrit Manager are not contiguous when sorted by ID, UUID or name.
Uh, that does sound odd...
I think the policy should just say "whoever has the permission to do it".
How about we make all groups be owned by Gerrit Manager? Then we will actually be able to explain who has the ability to do this.
I thought the idea was that no single group should have all the power, for security reasons. Wasn't that why the Administrators group was changed, after we had vandalism on Gerrit?
No. The built-in Administrators group has powers that shouldn't generally be wielded, and which we can't remove or restrict (but would almost never need to use). The idea was to reduce the number of accounts with world-ending powers; it wasn't related to the kinds of actions this policy covers.
So Gerrit Manager should indeed own all groups?
However Gerrit Managers don't have the ability to add people to the MediaWiki group. If you go with Gerrit Managers, we should also empower them to add/remove people from there or leave things as they are and let the LDAP group gerritadmin people to add people to the mediawiki group as it happens now AIUI.
Under the bulleted exceptions to Merging without review, there is a point that "A commit in Gerrit may have two authors" mentioning "an owner and a reviewer who uploads and amendment"; however, there is no exception for pair programming explicitly. I am not sure how common this is for MediaWiki core and Extensions (cf: scope?), but it is common for several repos I work in.
That is, a patch could be written during a hangout/pairing session between two people, uploaded by one, and merged by the same. As a recommendation, it could contain a Co-Authored-By: snippet in the commit or two Signed-off-by: tags.
Isn't there a risk that pair-programmed code suffers from the same individual "group" think that we worry about for sole-author patches? Do we want to encourage the practice of pair-programmed code being merged by one of said pair?
I think code review and pair programming both ensure that two people understand and endorse a particular patch. I view pair programming as a real-time review of code being written, whereas code review is asynchronous.
I think code review and pair programming are both equally subject to groupthink and/or power inequalities. I am unsure if one is more so than the other.
I agree that pair programming is kind of a live review process. There is no good way to represent shared authorship on gerrit, so one person should upload the patch, and the other can approve it right away. MAybe it would be nice to mention the fact that the patch was done in a pair programming session, but I don't think it's necessary.
In any case, the fact that there were "four eyes" on the code should be documented on gerrit. That's the essential part.
The document talks mostly about Mediawiki deployment and Mediawiki extensions, but there are more projects in Gerrit than this. I think it should be clarified whether these policies are to apply to them too and then they need to be written in a bit more generic way or just to "big Mediawiki" (i.e. Mediawiki plus extensions plus puppet, etc.) but not other code hosted on WMF Gerrit.
It's intended to apply to all projects that are in WMF Gerrit. There are some bits that are fairly specific to MediaWiki, do you think they should be removed?
FWIW, I initially had the same reaction, many of the arguments surrounding the importance of +2 make direct reference to the particular configuration of MediaWiki and extensions Gerrit and Jenkins setups.
Putting that aside, the section on merging without code review feels particularly prescriptive; however, it makes the point towards the end:
For extensions (and other projects) not deployed to the Wikimedia cluster, the code review policy is up to the maintainer or author of the extension.
I think I would prefer if that were easier to glean from an inspectional reading of the policy rather than a close reading.
Also, it's important to note that self-merge is common in some repositories (integration/config for instance), and there may be differences between projects in what exceptions there are to merging without code review; however, in these projects, +2 is still "a strong expression of trust", only the expectations are different.