On Fri, May 30, 2014 at 4:06 AM, Daniel Kinzler <daniel(a)brightbyte.de>
wrote:
Am 29.05.2014 21:07, schrieb Aaron Schulz:
Yes it was for auto-reviewing new revisions. New
revisions are seen as a
combination of (base revision, changes).
But EditPage in core sets $baseRevId to false. The info isn't there for the
"standard" case. In fact, the ONLY thing in core that sets it to anything
but false is commitRollback() , and that sets it to a value that to me
doesn't make much sense to me - the revision we revert to, instead of
either the revision we revert *from* (base/physical parent), or at least
the *parent* of the revision we revert to (logical parent).
I think you need to look again into how FlaggedRevs uses it, without the
preconceptions you're bringing in from the way you first interpreted the
name of the variable. The current behavior makes perfect sense for that
specific use case. Neither of your proposals would work for FlaggedRevs.
As for the EditPage code path, note that it has already done edit conflict
resolution so "base revision = current revision of the page". Which is
probably the intended meaning of false.
Of course, we need to check FlaggedRevs and other
extensions, but seeing
how this argument is essentially unused, I can't imagine how this change
could break anything for extensions.
Except FlaggedRevs.
--
Brad Jorsch (Anomie)
Software Engineer
Wikimedia Foundation