[05:01:01] * Krinkle spots sitespeed.io at https://crawler.ninja/files/floc-opt-out-sites.txt [05:01:09] It's a nice list .. [05:01:24] Context: https://scotthelme.co.uk/what-the-floc/ [06:34:42] Krinkle: I don't think we are changing for PG timestamps to string, at least not in the abstraction work. mwtimestamp abstract data type just becomes TIMESTAMPTZ in PG. Specially given that it's not part of the original RFC of making PG consistent [06:34:47] HTH [07:41:34] Krinkle: thanks for sharing, I didn't know about that list [10:35:38] Amir1: yeah, I noticed that but wonder why. I guess for abstract it's okay to handle later to not mix changes [10:46:56] exactly [10:47:37] reducing changes to minimum. Already PG had +200 schema changes because of abstraction [17:50:10] Krinkle: it's too bad dbal doesn't use BLOB for DECIMAL with sqlite. The sqlite NUMERIC() type is a lie (a lossy float)... [17:50:43] DECIMAL(29) would be smaller than char(22) and would fit 128 bit ids [17:51:23] I'd have to make an mwdecimal type to handle sqlite though [21:16:04] Krinkle: Each time you force-push I have to re-review from scratch as GH has no affordances for showing the diff between different git hashes. :-P [21:18:20] https://github.com/oyejorge/less.php/compare/1ae7037...2874fd3 [21:18:25] and yes, I made that by hand [21:18:35] also, it's funny that it works because I used the wrong org [21:18:54] Every time I ask GH for such links it panics and says no such commit exists. [21:19:35] hn. this link is bogus as well [21:19:41] Also that. [21:19:43] it's not showing a minimal diff at all [21:19:50] yeah, compare/*..* finds commits [21:19:54] Nope, it's just hopeless. [21:19:57] and then combines the diff of those whole commits [21:20:09] it's not doing a tree diff like git diff .x..y would [21:20:17] because you know, reinventing the wheel sells better [21:20:18] E.g. the link to "changes since I last looked" is https://github.com/wikimedia/less.php/pull/58/files/a486d78b9bd16b72f237fc6093aa56d69ce8bd13..HEAD [21:20:25] yeah [21:20:27] those never worked afaik [21:20:32] not sure what they smoked when they shipped that [21:20:37] Which helpfully says that force-pushes 'sometimes' obliterates things. [21:20:45] oh I guess that might work for non-force ones [21:20:53] I can't wait to move to GitLab and have an upstream I can at least inspect and potentially fix. [21:20:55] but also you can click on the commit hashees in that case and get the same diff [21:20:59] Indeed. [21:21:27] I'll try to remember to use squash merge instead, and manually keep separate branches. [21:21:50] Yeah. [21:21:58] (also, reminds me of the impending doom on gitlab where we'll presumably have to leave messages all over to "not mege this PR before the subset of this PR over [here]" [21:22:00] But it's a habit from gerrit that's hard to drop. [21:22:15] No no, just lots of branches called DNM-OMG-NO [21:22:31] squash merge for one off PRs is okay I'm not too worried about that, other than learning habits indeed [21:22:39] the issue is when you actually want separate PRs and review cycles [21:22:45] and e.g. keep stacking on top while review happens [21:22:52] you know, "productivity" [21:22:53] Indeed. GH's answer is "no". [21:23:10] Because who'd ever have a non-trivial codebase, or downstream users? I mean, seriously. [21:23:51] GitHub is for small projects where you're the only maintainer, or a big project and designate a few who are on merge duty and do all the rebasing and proper ordering for you. something we stopped doing after we left SVN. [21:24:18] Clearly we should go back to having a trunk-meister. [21:24:58] every project I contribut to on Github or Gitlab where I want to submit two patches, I generally can't. Either you end up with a single PR and the author saying "please split up" (not realizing github doesn't make that easy because they never do it themselves and just self-merge their work), or you keep things separate and then then author is confused why you have a PR that includes the work of antoher PR. [21:25:07] Given we averaged ~ one commit per 12 minutes of business hours (16 hours a day, 5 days a week) in the last quarter, per Greg's number crunching… [21:25:10] or what I usually do, is I submit one PR, and then wait a month before doing the rest. [21:25:26] Yeah, it's such a productivity sink. Oh well. [21:25:53] but don't worry, well make a custom made UI on top of this that's better than Gerrit and not at all non-standard. [21:26:22] Oh gods. [21:26:28] I can foresee it now. [21:33:05] * bd808 munches popcorn and agrees that there are big workflow changes for everyone coming with gitlab [21:33:54] I have personally destroyed multiple github repos by trying to treat them like I was using gerrit [21:41:19] I'd really like to learn if and how others are able to do this at scale. I've not seen solutions so far. People seem to either have projects where merging isn't critical (e.g. just leave the PR until we're doing a release, not application deployment), or smaller projects tightly maintained by a team where you just don't do the next bit of work yet until it's merged and maybe do something else meanwhile, or e.g. wait until someone has [21:41:19] time to review your stack in its entirety and then merge it in its entirety. [21:41:53] I don't know how people would e.g. land part of a stack progressively as we do today, or some equivalent of that. For one, CI tends to not run on that. and there's certainly no UI for it. [21:42:45] I'm sure we'll find a way, but whether people would be happy with it, and resulting in good health (with our already pretty poor practices), I'm not sure. We'll certainly need a bit more discipline, I think. [21:43:40] Like, if we have to land patches with the command-line by submitting sub parts to a temporary branch, waiting for CI, and then pushing to master, if it satisfies the status checks, presuably people that find Gerrit annoying would find that more annoying. [21:43:43] Anyway :) [21:48:43] re custom made ui... https://github.com/realyze/pr-train is the sort of thing that I'd bet on showing up (hopefully not in the form of a giant pile of node dependencies) [21:51:37] lol -- https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_dependencies.html -- "Merge requests dependencies are a PREMIUM feature" [21:51:57] * bd808 slinks away [21:55:39] bd808: yeah, this is somewhat equivalent to depends-on, which is for changes to other repositories. I don't think it applies within the repo, but if it does, it would presumably just cherry-pick them on top, which means the diffs can't clash. [21:56:13] local testing would be tediuous, checking out one and then cherry-picking the other(s) [21:56:31] and then un-basing into separate branches without any parents before pushing to the pr branches [21:57:32] I didn't predict myself as favouring gerrit but over time I've come to think of it as not so much "for Gerrit" as "for Git / against GitHub's engagement model" [21:58:02] I do regularly maintain projects on github, and have an incentive to find a way within the system [21:58:34] I haven't and haven't found others finding a way either even those who haven't been biased by Gerrit. [21:59:13] instead I end up giving people access and doing post-push review here and there, it's jsut not respectful of their time otherwise imho. [23:20:23] Krinkle: For T146416 shipping a hard-deprecation now sounds fine, but, err, what exactly are we hard-deprecating? Just when calling toString with format !== parse? We already throw an exception if it's null. [23:20:24] T146416: Message -> string transformations should not affect each other - https://phabricator.wikimedia.org/T146416 [23:21:41] Oh, forgive me, we create a LogicException and then warn about it. [23:21:47] OK, no worries. [23:22:02] James_F: Hm. no you're right, we do more [23:22:08] __toString() always uses PARSE [23:22:14] regardless of what the last called format was [23:22:16] even if not null [23:22:23] we still ignore it and always safely return escaped [23:22:28] Yeah. [23:22:40] so msg->plain() and then echo $msgObj; will not use plain, but parse/escape [23:22:43] But we do re-use $this-format() [23:22:44] so that looks like it's resolved [23:23:01] Dropping the re-use is still outstanding? [23:23:17] And just change the default $format to FORMAT_PARSE? [23:23:50] well.. the issue is when $msgObj is silently converted to a string via casting, echo'ing concat, etc where __toString() gets magically called [23:23:54] that is completely fixed afaik [23:24:01] Yeah. [23:24:07] the sticky format does apply to explicitly/manaully calling toString() [23:24:28] Since REL1_28 indeed. [23:24:36] Only to implicitly. [23:25:26] I think this task is resolved. But I wouldn't oppose *also* turning that if (format=null) branch in toString() to a wfDeprecated() call, and then changing the condition such that the deprecation is not only triggered for format null, but for all public calls. [23:25:46] e.g. we can temporary add a second $internal=false parameter and let our plain()/parse() etc methods pass that, and if !internal, always deprecate [23:26:52] so we'd be deprecating the $msg->toString() as a public method, e.g. discouraging people from doing void $msg->plain(); return $msg->toString(); instead of simply return $msg->plain(); [23:27:01] which indeed afaik is never done intentionally anywhere [23:27:11] * Krinkle whips up a patch [23:56:02] James_F: noticing my git commit parent is `Merge "es6-polyfills: Remove deprecated alias "es6-promise""` [23:56:12] I hadn't noticed you already merged that stack. Thanks! [23:56:30] Krinkle: Thanks for working on it. And RIP my most recent MW core RL module. ;-) [23:58:35] curious how CI will fare on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/681818/ [23:58:48] I'm really hoping we're not calling toString() anywhere outside its own unit test [23:59:33] I'll tag T146416 once this first CI run is done (forgot to do earlier but dont' want to abort jobs) [23:59:33] T146416: Message -> string transformations should not affect each other - https://phabricator.wikimedia.org/T146416