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).
Also, if you want (base revision, changes), you would use $oldid in
doEditContent, not $baseRevId. Perhaps it's just WRONG to pass $baseRevId to the
hooks called by doEditCOntent, and it should have been $oldid all along? $oldid
is what you need if you want to diff against the previous revision - so
presumably, that's NOT what $baseRevId is.
If baseRevId is always set to the revision the user
started from it would
cause problems for that extension for the cases where it was previously
"false" means "don't check", I suppose - or "there is no
base", but that could
be identified by the EDIT_NEW flag.
I'm not proposing to change the cases where baseRevId is false. They can stay as
they are. I'm proposing to set baseRevId to the revision the user started with,
OR false, so we can detect conflicts safely & sanely.
It would indeed be useful to have a casRevId value
that was the current
revision at the time of editing just for CAS style conflict detection.
Indeed - but changing the method signature would be painful, and the existing
$baseRevId parameter does not seem to be used at all - or at least, it's used in
such an inconsistent way as to be useless, of not misleading and harmful.
For now, I propose to just have commitRollback call doEditContent with
$baseRevId = false, like the rest of core does. Since core itself doesn't use
this value anywhere, and sets it to false everywhere, that seems consistent. We
could then just clarify the documentation. This way, Wikibase could use the
$baseRevId value for conflict detection - actually, core could, and should, do
just that in doEditContent; this wouldn't do anything in core until the
$baseRevId is supplied at least by EditPage.
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.