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. "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. -- daniel _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
