Page MenuHomePhabricator

Gerrit can be bypassed obscuring code changes
Closed, ResolvedPublic

Description

I am not willing to run code that is both unreviewed and not seen by me.


Version: unspecified
Severity: blocker

Details

Reference
bz36927

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 12:29 AM
bzimport added a project: Gerrit.
bzimport set Reference to bz36927.

No it's not: "anyone can suggest a change and submit a pull request, and it will automatically be approved and merged." This is not happening. It's just going directly into the master.

This is by design. Not everyone cares to have their code reviewed. Wikimedia extensions are required to be reviewed, of course, but we don't force everyone to wait for reviews.

You're free to not use any extension that is unreviewed. Remember, though, that in the past this was the norm.

(In reply to comment #3)

This is by design. Not everyone cares to have their code reviewed. Wikimedia
extensions are required to be reviewed, of course, but we don't force everyone
to wait for reviews.

You're free to not use any extension that is unreviewed. Remember, though, that
in the past this was the norm.

I think you're completely missing the point, Ryan. In the past, there used to be a log of all commits (Special:CodeReview/MediaWiki). There also used to be a mailing list in which all commits were pushed to those interested (mediawiki-cvs).

Both a no longer true. People being able to go around Gerrit means that they have no way to subscribe to changes made in git.

git log doesn't work? It's technically impossible to override previous revisions in git, there is always a log

(In reply to comment #5)

git log doesn't work? It's technically impossible to override previous
revisions in git, there is always a log

This works for one repo, but currently there is no place where you can get an overview of what is happening to core and extensions, like you did on the old CR. I fully agree with Siebrand that we ought to have something like this.

One way to fix this is to have gerrit display things that got directly pushed to master in the "merged" list. Another way to fix it is to have "automatic merge" functionality on gerrit, so people that do not wish to have to approve their own code or wait for a very long time before someone bothers to look at their change to $randomExtension can just push using git review.

(In reply to comment #6)

One way to fix this is to have gerrit display things that got directly pushed
to master in the "merged" list. Another way to fix it is to have "automatic
merge" functionality on gerrit, so people that do not wish to have to approve
their own code or wait for a very long time before someone bothers to look at
their change to $randomExtension can just push using git review.

Both solutions sound like things to be fixed upstream.

Removing "upstream" keyword. Requirements for using it are not met.

Chad, has this already been fixed (as previously announced on wikitech-l)?

I don't see a reason for High/Blocker here, hence removing.

My initial comment isn't a good enough reason?

Code is made to be used, gigegat is made to allow code review; this bug prevents both.

For Translatewiki.net I am working with hundreds of extensions. Every extension that bypass Gerrit is lost for translatation. We have no chance to review them.

Chad, can you please provide an update? IIRC you've told me in the past this is a fairly trivial configuration change.

This has been changed for all extension master branches, thanks for reminding me.

Current project.config for mediawiki/extensions: https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/extensions.git;a=blob;f=project.config;hb=refs/meta/config