[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-08-21 Thread gerritbot
gerritbot added a comment. Change 445200 merged by jenkins-bot: [mediawiki/core@master] [MCR] Render multi-slot diffs https://gerrit.wikimedia.org/r/445200TASK DETAILhttps://phabricator.wikimedia.org/T194731EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To:

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-08-14 Thread Tgr
Tgr added a comment. Moving back to review as all dependencies (452470, 452413) are also up for review.TASK DETAILhttps://phabricator.wikimedia.org/T194731EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: TgrCc: Jdlrobson, pmiazga, Aklapper, gerritbot, daniel,

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-08-13 Thread Tgr
Tgr added a comment. Thanks! In T194731#4499846, @pmiazga wrote: MobileFrontend uses different DiffEngine (it's hardcoded), this is something you have to talk to Reading Web Yeah, that's tracked in T201842. I also remember there is a work on a different engine that nicely shows when stuff

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-08-13 Thread gerritbot
gerritbot added a comment. Change 441925 abandoned by Gergő Tisza: [WIP][MCR] Show diffs for all slots Reason: Abandoning in favor of https://gerrit.wikimedia.org/r/c/mediawiki/core/ /445200 https://gerrit.wikimedia.org/r/441925TASK DETAILhttps://phabricator.wikimedia.org/T194731EMAIL

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-08-13 Thread daniel
daniel added a comment. In T194731#4498498, @Tgr wrote: Went through the high and medium priority steps of the test plan. MobileFrontend diffs break (use TextSlotDiffRenderer instead of InlineDifferenceEngine). MobileFrontend hand-creates the InlineDiffRenderer so it does not get used for the

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-08-06 Thread Tgr
Tgr added a comment. Test plan: https://www.mediawiki.org/wiki/User:Tgr_(WMF)/DifferenceEngine/test_planTASK DETAILhttps://phabricator.wikimedia.org/T194731EMAIL PREFERENCEShttps://phabricator.wikimedia.org/settings/panel/emailpreferences/To: TgrCc: pmiazga, Aklapper, gerritbot, daniel, Gaboe420,

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-07-19 Thread gerritbot
gerritbot added a comment. Change 441924 abandoned by Gergő Tisza: [WIP][MCR] Split DifferenceEngine into page-level and slot-level part Reason: Abandoning in favor of https://gerrit.wikimedia.org/r/c/mediawiki/core/ /445200 https://gerrit.wikimedia.org/r/441924TASK

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-07-11 Thread gerritbot
gerritbot added a comment. Change 445200 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza): [mediawiki/core@master] [WIP] Render multi-slot diffs https://gerrit.wikimedia.org/r/445200TASK DETAILhttps://phabricator.wikimedia.org/T194731EMAIL

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-07-10 Thread Tgr
Tgr added a comment. Thanks, that's nicer than what I wrote for option 2. The only potential problem I can see is when something uses createDifferenceEngine to get an instance, and the content handler or GetDifferenceEngine hook handler subclasses it, and the subclass overrides something in a way

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-07-10 Thread daniel
daniel added a comment. @Tgr thank you for doing the survey! I'm ok with doing the sub-classing approach as an intermedia solution if we can't find a better alternative that can be done quickly. How about this option: Declare DifferenceEngine to be the page-level handler Deprecate all

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-07-10 Thread daniel
daniel added a comment. Copy of @Tgr's comment at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/441924#message-edcc2029a8d911da3ec680fb373cd10f9b0323a2, for further discussion here: So one constraint to refactoring here is what callers expect a DifferenceEngine subclass. That can happen two

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-07-10 Thread daniel
daniel added a comment. A copy of my response to @Tgr at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/441924#message-988f7c87660b1daba3ca441b29a92f4c6c036a47, copied for the sake of further discussion here: You wrote: To maintain a semblance of B/C, PageDifferenceEngine becomes a parent

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-07-10 Thread daniel
daniel added a comment. Copy of the commit message of https://gerrit.wikimedia.org/r/c/mediawiki/core/+/441924 as written by @Tgr, for the sake of discussion here: [WIP][MCR] Split DifferenceEngine into page-level and slot-level part In preparation for showing multi-slot diffs, split out from

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-06-25 Thread gerritbot
gerritbot added a comment. Change 441924 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza): [mediawiki/core@master] [WIP][MCR] Split DifferenceEngine into page-level and slot-level part https://gerrit.wikimedia.org/r/441924TASK

[Wikidata-bugs] [Maniphest] [Commented On] T194731: Show diffs for all slots [MCR]

2018-06-25 Thread gerritbot
gerritbot added a comment. Change 441925 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza): [mediawiki/core@master] [WIP][MCR] Show diffs for all slots https://gerrit.wikimedia.org/r/441925TASK DETAILhttps://phabricator.wikimedia.org/T194731EMAIL