Page MenuHomePhabricator

Florianschmidtwelzow +2 for CodeMirror
Closed, InvalidPublic

Description

With T95041: Deploy CodeMirror extension to beta cluster it's planed to deploy CodeMirror to beta labs (and maybe to production sometimes, too). Even if this wouldn't be the case, it would be great to have a second reviewer for CodeMirror, so @Pastakhov doesn't need to self merge his own changes (which wouldn't be permitted, if the extension is deployed to WMF servers). I'm familar with most of the code base and would be happy to support the extension by reviewing changes :)

Event Timeline

Florian raised the priority of this task from to Needs Triage.
Florian updated the task description. (Show Details)
Florian added subscribers: Florian, Pastakhov.

I do not mind if someone will approve my own patches to CodeMirror, if it is required for security purposes.
And I would be happy if @Florian will support the extension by reviewing changes, but I doubt that he will fit this role better than me.
Why do you trust @Florian more than me?

When the extension gets deployed you won't be allowed to +2 your own patches, see https://www.mediawiki.org/wiki/Gerrit/%2B2

@Pastakhov That's not because anyone doesn't trust you, you're the extension owner, so I'm pretty sure, that you know this extension much more then me. But the goal of a code review is, that we don't merge code that doesn't fit our requirements in any way (e.g. coding conventions or how a function is implemented from a performance or/and security point of view among other things). One of the basics for a successful code review is, that the person who wrote the code doesn't review it. See, for more information, https://www.mediawiki.org/wiki/Gerrit/%2B2#.2B2_is_for_code_review.2C_not_merging_your_own_stuff

I hope that you don't misunderstand this request in any way, sorry, if this happens already :]

EDIT:
Krenair was faster :P

@Krenair, @Florian: thanks for the link. I didn't fully understand this request. It's clear now :-)