[00:21:22] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page, 13Patch-For-Review, 05WMF-deploy-2016-12-13_(1.29.0-wmf.6): Implement functionality for RC page 'Contribution Quality' filters (ORES) - https://phabricator.wikimedia.org/T149734#2895211 (10Etonkovidova) @jmatazzo... [00:24:52] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page, 13Patch-For-Review, 05WMF-deploy-2016-12-13_(1.29.0-wmf.6): Implement functionality for RC page 'Contribution Quality' filters (ORES) - https://phabricator.wikimedia.org/T149734#2895214 (10jmatazzoni) > My concer... [00:26:33] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page, 07Tracking: Implement enhanced Recent Changes filters (and make them work with the new UI) - https://phabricator.wikimedia.org/T144451#2895216 (10jmatazzoni) [00:26:35] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page, 13Patch-For-Review, 05WMF-deploy-2016-12-13_(1.29.0-wmf.6): Implement functionality for RC page 'Contribution Quality' filters (ORES) - https://phabricator.wikimedia.org/T149734#2895215 (10jmatazzoni) 05Open>0... [00:40:41] If we need to reopen the thresholds discussion, then let's please do it in T149761, if you don't mind. [00:40:42] T149761: Fine-tune and finalize ORES score ranges for the Quality and Intent filters - https://phabricator.wikimedia.org/T149761 [00:41:02] I'll reopen it. I knew it was optimistic to close. :C [00:41:32] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page, 05MW-1.29-release-notes, 13Patch-For-Review, 05WMF-deploy-2016-12-13_(1.29.0-wmf.6): Implement functionality for RC page 'User Intent' filters (ORES) - https://phabricator.wikimedia.org/T149853#2895280 (10jmataz... [00:41:34] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page, 13Patch-For-Review, 05WMF-deploy-2016-12-13_(1.29.0-wmf.6): Implement functionality for RC page 'Contribution Quality' filters (ORES) - https://phabricator.wikimedia.org/T149734#2895281 (10jmatazzoni) [00:41:37] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements: Fine-tune and finalize ORES score ranges for the Quality and Intent filters - https://phabricator.wikimedia.org/T149761#2895279 (10jmatazzoni) 05Resolved>03Open [01:28:32] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 07Design, 13Patch-For-Review, 05WMF-deploy-2017-01-03_(1.29.0-wmf.7): Notices tray icon with 99+ needs more space - https://phabricator.wikimedia.org/T142454#2895372 (10Etonkovidova) Checked in betalabs - IE11, Windows 7, Chrome... [06:52:09] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 07Design, 13Patch-For-Review, 05WMF-deploy-2017-01-03_(1.29.0-wmf.7): Notices tray icon with 99+ needs more space - https://phabricator.wikimedia.org/T142454#2895547 (10Pginer-WMF) >>! In T142454#2895372, @Etonkovidova wrote: > @... [08:54:24] (03CR) 10Zfilipin: "#31 creates users and pages using random strings generated by securerandom." [extensions/Echo] - 10https://gerrit.wikimedia.org/r/313221 (https://phabricator.wikimedia.org/T146916) (owner: 10Etonkovidova) [10:12:56] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 07User-notice-collaboration: Special:Notifications is too slow to open - https://phabricator.wikimedia.org/T153011#2895735 (10Aklapper) https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=756146851#Not... [12:32:16] 06Collaboration-Team-Triage, 07Design, 07Tamil-Sites: Feedback text overlaps with personal bar on Tamil Wikipedia - https://phabricator.wikimedia.org/T36681#2895906 (10MarcoAurelio) [12:32:24] 06Collaboration-Team-Triage, 07I18n, 07Tamil-Sites: Increase allowable feedback text length - https://phabricator.wikimedia.org/T36586#2895908 (10MarcoAurelio) [14:56:59] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page: Update RC page results without reloading the page (AJAX) when filters are changed - https://phabricator.wikimedia.org/T153949#2896180 (10Catrope) [15:23:08] stephanebisson: I gave your suggestion for AJAX a try and it seems to work quite well: https://gerrit.wikimedia.org/r/#/c/328676 [15:23:24] RoanKattouw: yeah, just looked at it [15:23:28] simple enough [15:25:22] Yeah [15:26:08] The only trap I fell into was that onPopState indirectly triggers pushState (via updating the filters) and that breaks the back button [15:26:42] But I remembered that Timo had to solve the same problem in VE code that I reviewed, all you need is a "did this come from a popstate event" parameter / state variable [15:27:18] that's what the tag is for? [15:32:15] No, not quite [15:32:36] The tag is so that if there's other code on the page also using pushState(), popstate events that pop to their states are skipped [15:33:00] It may well be unnecessary in our case, I copied it from VE where it's probably more necessary [15:34:03] On the one hand it's a nice defense against pushState interaction bugs where we try to interpret a URL built by some other piece of code, but on the other hand it also has issues [15:35:48] The main problem being that the initial state doesn't have our tag, and so we use replaceState() to tag the initial state with our tag (also a trick I stole from Timo's VE code), but of course that's something that only one tool can do [15:36:02] If multiple things try to do that on the same page, one of them will break the other [15:36:11] So I'm not totally convinced that the tag thing is a good idea just yet [15:36:59] A better use of the state object might be to store filter state or something so that we don't have to re-parse the URL [15:37:08] That seems to be what MDN recommends it for [15:38:40] Anyway, moral of the story: the pushState/popstate API is an annoying one [15:38:50] (I wish the casing there was a typo) [17:01:39] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 10Thanks: Move Thanks icons to Thanks repo - https://phabricator.wikimedia.org/T151768#2896434 (10Volker_E) @MtDu Any updates or more information you'd need? [17:03:07] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 10Thanks: Move Thanks icons to Thanks repo - https://phabricator.wikimedia.org/T151768#2896437 (10Volker_E) [17:03:10] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 10UI-Standardization, 13Patch-For-Review, 03UI-Standardization-Kanban: Echo Notifications Thank You uses outdated PNG icon instead of SVG - https://phabricator.wikimedia.org/T149352#2896436 (10Volker_E) [17:04:23] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 10UI-Standardization, 13Patch-For-Review, 03UI-Standardization-Kanban: Echo Notifications Thank You uses outdated PNG icon instead of SVG - https://phabricator.wikimedia.org/T149352#2749556 (10Volker_E) a:05Volker_E>03MtDu [17:07:08] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 10Thanks: Move Thanks icons to Thanks repo - https://phabricator.wikimedia.org/T151768#2896441 (10MtDu) Sorry for pushing this off @Volker_E @Catrope . I remember taking a brief look at this, and seeing it was a bit more complicated... [19:19:54] mooeypoo: So, just to check that we're on the same page, when we call something a "parameter" it should always be a string, right? [19:20:45] So for example getFiltersFromParameters should take arbitrary strings as its input, because callers will just grab mw.Uri stuff and pass it right through [19:20:55] Does that align with your tihnkin? [19:21:00] *thinking, jeez [19:28:09] hey, flow-sters [19:28:21] ...Update 'FlowBetaFeatureDisable' already logged as completed. [19:28:21] ...Update 'FlowPopulateRefId' already logged as completed. [19:28:21] PHP Fatal error: Class 'Pimple\Container' not found in /home/cananian/Projects/Wikimedia/Flow/includes/Container.php on line 5 [19:28:28] ^ does that look familiar to y'all? [19:28:58] (after running `git pull` to update my rather-ancient local version of Flow) [19:32:25] RoanKattouw, yes [19:32:38] RoanKattouw, parameters are what we send the back-end AND what we display in the URL [19:32:43] cscott: composer update [19:32:50] so they're an absolute value, not a symbolic false/true like the filters [19:32:54] if that makes sense [19:32:57] Right [19:33:04] And those values could be total garbage [19:33:09] They're user input [19:33:13] RoanKattouw: thanks. I guess the top-level `composer update` doesn't recurse into extensions. [19:33:20] Hmm apparently not [19:33:29] RoanKattouw, I have to say, though, I am starting to wonder about the way I do parameter validation in general. It is okay for now, but we'll likely want to completely take it out once the back end does it for us [19:33:33] I didn't realize it didn't because I encountered this problem separately and ran it separately [19:33:39] RoanKattouw, right, exactly. [19:33:51] OK, cool [19:34:48] Then I think what I'm doing in my AJAX patch makes sense [19:34:57] RoanKattouw, there's a thing about how we write things now and usually refactor in a while when we see better how all parts combine, so I think we'll have to do that at some point with the parameter-validation method *and* probably with the group representation [19:35:05] at some point it might just make sense to create a group data model [19:35:10] OK [19:35:17] though I don't want to hold off the current patches for that [19:35:30] I don't want to fall into the "make things better" loop forever [19:35:32] Or maybe not, if we end up not doing strict validation in the frontend [19:35:41] Sure, fair enough [19:35:49] Right, that's why I am slightly apprehensive fixing things *right now* [19:35:58] It's just that right now I had to change code from your patches to make the AJAX patch even work [19:36:00] Only a couple, but still [19:36:06] So we should fix those things at least [19:36:08] we don't yet know exactly how we're going to deal with validation, and most of the slightly more problematic aspects are for that [19:36:15] why? [19:36:37] As I commented in Gerrit, the discussion about group data models and the duplication of validity checks is all deck chairs on the Titanic stuff since you pointed out we may not even have this validation code in JS for long [19:36:46] mooeypoo: Really that was all Number() stuff though [19:36:53] yeah [19:36:53] https://gerrit.wikimedia.org/r/#/c/328676/1/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js [19:36:54] okay [19:36:59] yeah I'm not happy about that either [19:37:04] codesmell pretty bad on those [19:37:19] but the parameter stuff that is stored is MOSTLY validation anyways [19:37:26] which, hopefully, we could get rid of soon [19:37:32] Right, exactly [19:37:36] I mean, ideally, we wouldn't have parameters at all stored [19:37:38] So if it's going to die, let's not worry about it too much for now [19:37:40] the system shouldn't give a crap about it [19:37:46] Yup agreed [19:37:54] Parameters in, conceptual values out, store those [19:37:55] you have the "getParametersFromFilters" that just does it for you on-the-fly when yo uneed it, and that's enough [19:37:59] right [19:38:10] the only reason I stored them is because of the issues with validation and making sure we spot errors [19:38:10] OK so I think we're in violent agreement [19:38:15] +1 [19:38:30] 1) don't worry about the validation stuff, as long as it kind of works but is smelly it doesn't matter, it's probably gonna be removed soon [19:38:38] RoanKattouw, I do think I may be missing some unit tests for the parameter-validation stuff though [19:38:45] 2) fix the other issues and hit the +2 button ASAP [19:38:50] * mooeypoo nods [19:38:53] awesome -- doing that now [19:39:45] RoanKattouw, btw, the parameter thing is why I tried my best to always state clearly whether we're dealing with parameters (user-input, values as strings, representation in the URL) vs filters (false/true states in groups) [19:39:53] I did that throughout the comments on the unit tests, too [19:40:37] Right [19:41:14] That wasn't clear to me in some places but I see how it all works now [19:42:44] Hmm the unit tests don't exercise the string input case AFAICT [19:42:56] If I have time this evening I might write some more tests [19:43:02] First going to catch up on your responses to the other patches [19:44:47] RoanKattouw, I think part of that reason is that I changed the way params worked a bunch of times and may have left some confused code in the midst? [19:45:13] I mean, they started out as false/true and then I realized I have bad-params I should represent, and also non-booleans (in the case of the other group) etc, so I changed that twice [19:46:43] Yeah probably [19:55:23] RoanKattouw, in some aspects I am a little meh having stuff that codesmell in a brand new architecture, but in this case, I think we run the risk of loop-de-loop-fixing AND if we all know that the optimization-parameter-part will go away, it might just be okay to do. [19:55:52] I do agree about not being happy with the way it is, though. [19:56:32] If we thought it should stay, I'd argue for reorganization of the way it works - but that is probably a waste of time for this ... I only added it in as a "temporary measure" when we talked about it in the Engineering meeting about param/filter problems. [19:59:17] mooeypoo: Alternatively maybe we can rip out that part and move it to its own patch? [19:59:33] "it" being the impossible parameter detection thing [20:07:01] RoanKattouw, hm, I guess, though again, I'm wondering if that will just delay things further unnecessarily? [20:07:13] It's pretty woven in in the later patches, too :\ [20:09:59] RoanKattouw, so, my worry right now is, again, delaying the patches a lot, especially considering we have a fairly large patch-stack, and rebasing is annoying as hell as it is :\ [20:10:15] Sure [20:10:18] That's true [20:10:25] At this point it's less work to not rip it out [20:10:30] yeah [20:10:38] though I am torn - I really dislike the param behavior thing [20:10:59] but I think it might be not that big of a deal to rip it out completely if/when we get rid of the validation [20:11:21] also ,btw, we COULD do a partial rip-out soon by getting rid of the INTERNAL validation and moving it to the outside [20:12:00] so like, insead of the viewmodel constantly validating to mark the groups + add "invalid" label param, we can just take that out completely in a followup patch, and instead validate *once* on init, and add a red box warning above the UI [20:12:17] Then "just" fix the params and give the system normalized data. [20:12:29] That will take out all the stupid codesmelly stuff out [20:12:35] Hmm yeah that's probably a better approach to validation regardless [20:12:51] well, we were saying that we wanted the problem-group to be marked, so that's why I went into the view model [20:13:12] but since the plan now is to have it done back-end anyways, then the fallback-UI behavior can just be the simple one [20:14:20] OK, I'm going to fix the patches re your comments. It's already pretty confusing because some of those comments are in various underlying patches [20:14:24] we can deal with the above later [20:14:36] Yeah, good idea [20:52:17] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016): [minor-regression] Notification counter '99+' overlaps mw-echo-notifications-badge - https://phabricator.wikimedia.org/T153976#2897110 (10Etonkovidova) [20:54:37] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 07Design, 13Patch-For-Review, 05WMF-deploy-2017-01-03_(1.29.0-wmf.7): Notices tray icon with 99+ needs more space - https://phabricator.wikimedia.org/T142454#2897129 (10Etonkovidova) Thx @Pginer-WMF . I filed {T153976} [21:11:38] (03PS1) 10Sbisson: Use the right counter for the right badge [extensions/Echo] - 10https://gerrit.wikimedia.org/r/328730 (https://phabricator.wikimedia.org/T142454) [21:15:25] 06Collaboration-Team-Triage, 10Flow: 1.28 and master versions of Flow aren't working - https://phabricator.wikimedia.org/T153979#2897193 (10Innosflew) [21:16:20] 06Collaboration-Team-Triage, 10Flow: 1.28 and master versions of Flow aren't working - https://phabricator.wikimedia.org/T153979#2897207 (10Innosflew) [21:16:36] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016): [minor-regression] Notification counter '99+' overlaps mw-echo-notifications-badge - https://phabricator.wikimedia.org/T153976#2897110 (10SBisson) I don't quite understand this ticket. The increased overlap of the counter over the icon is not a regressio... [21:17:25] 06Collaboration-Team-Triage, 10Flow: 1.28 and master versions of Flow aren't working - https://phabricator.wikimedia.org/T153979#2897193 (10Innosflew) [21:18:10] 06Collaboration-Team-Triage, 10Flow: 1.28 and master versions of Flow aren't working - https://phabricator.wikimedia.org/T153979#2897193 (10Innosflew) [21:18:27] 06Collaboration-Team-Triage, 10Flow: 1.28 and master versions of Flow aren't working - https://phabricator.wikimedia.org/T153979#2897217 (10Innosflew) [21:18:51] 06Collaboration-Team-Triage, 10Flow: 1.28 and master versions of Flow aren't working - https://phabricator.wikimedia.org/T153979#2897193 (10Innosflew) [21:22:53] (03CR) 10Catrope: [C: 032] Use the right counter for the right badge [extensions/Echo] - 10https://gerrit.wikimedia.org/r/328730 (https://phabricator.wikimedia.org/T142454) (owner: 10Sbisson) [21:31:13] (03Merged) 10jenkins-bot: Use the right counter for the right badge [extensions/Echo] - 10https://gerrit.wikimedia.org/r/328730 (https://phabricator.wikimedia.org/T142454) (owner: 10Sbisson) [22:52:23] quiddity: hmm... creating a new topic on a Flow board is never a subject for Page Curation (Special:NewPagesFeed) ? In any wiki, in any namespace? [23:08:43] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Notifications, 07Design, 13Patch-For-Review, 05WMF-deploy-2017-01-03_(1.29.0-wmf.7): Notices tray icon with 99+ needs more space - https://phabricator.wikimedia.org/T142454#2897473 (10jmatazzoni) 05Open>03Resolved [23:10:16] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Flow, 13Patch-For-Review, 05WMF-deploy-2017-01-03_(1.29.0-wmf.7): New topic notification double parses? - https://phabricator.wikimedia.org/T153605#2885500 (10Etonkovidova) Checked in betalabs - {{tlx|speedy}} and {{tlx|delete}} are not expanded... [23:49:55] 06Collaboration-Team-Triage (Collab-Team-Q2-Oct-Dec-2016), 10Edit-Review-Improvements-RC-Page, 06Editing-Analysis, 13Patch-For-Review: Instrument how often various filters on Special:Recentchanges are used - https://phabricator.wikimedia.org/T144331#2897569 (10jmatazzoni) @Mooeypoo, I just wanted to check... [23:57:41] etonkovidova, not currently, no. (NewPagesFeed is only available at Enwiki) [23:58:39] quiddity: thx - I thought so too. Just wanted to double-check :)