Last modified: 2013-04-25 12:05:30 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T38927, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 36927 - Gerrit can be bypassed obscuring code changes
Gerrit can be bypassed obscuring code changes
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
unspecified
All All
: High blocker with 1 vote (vote)
: ---
Assigned To: Chad H.
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-17 08:52 UTC by Niklas Laxström
Modified: 2013-04-25 12:05 UTC (History)
14 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Niklas Laxström 2012-05-17 08:52:19 UTC
I am not willing to run code that is both unreviewed and not seen by me.
Comment 1 p858snake 2012-05-17 08:53:53 UTC
I believe this is design. See: https://www.mediawiki.org/wiki/Git/Workflow#Other_MediaWiki_extensions
Comment 2 Niklas Laxström 2012-05-17 08:55:21 UTC
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.
Comment 3 Ryan Lane 2012-05-17 15:05:34 UTC
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.
Comment 4 Siebrand Mazeland 2012-05-18 08:03:49 UTC
(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.
Comment 5 Peter Bena 2012-05-18 10:59:17 UTC
git log doesn't work? It's technically impossible to override previous revisions in git, there is always a log
Comment 6 Jeroen De Dauw 2012-05-18 12:37:37 UTC
(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.
Comment 7 Mark A. Hershberger 2012-05-18 14:02:27 UTC
(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.
Comment 8 Siebrand Mazeland 2012-05-24 13:16:28 UTC
Removing "upstream" keyword. Requirements for using it are not met.
Comment 9 Nemo 2012-10-03 06:42:17 UTC
Chad, has this already been fixed (as previously announced on wikitech-l)?
Comment 10 Andre Klapper 2012-10-23 05:15:38 UTC
I don't see a reason for High/Blocker here, hence removing.
Comment 11 Niklas Laxström 2012-10-23 06:51:35 UTC
My initial comment isn't a good enough reason?
Comment 12 Nemo 2012-10-23 07:32:28 UTC
Code is made to be used, gigegat is made to allow code review; this bug prevents both.
Comment 13 Raimond Spekking 2012-10-23 07:42:38 UTC
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.
Comment 14 Siebrand Mazeland 2012-10-23 09:14:48 UTC
Chad, can you please provide an update? IIRC you've told me in the past this is a fairly trivial configuration change.
Comment 15 Chad H. 2012-10-23 11:45:20 UTC
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
Comment 16 Siebrand Mazeland 2012-10-23 13:36:16 UTC
Thanks Chad!

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links