On Fri, May 30, 2014 at 4:06 AM, Daniel Kinzler daniel@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.