[14:30:24] duesen: Sigh. I wish you'd have given me a chance to review https://gerrit.wikimedia.org/r/506945 before you +2ed it. [14:31:34] ApiBase already has too many random utility methods in it, and the claim that a call to a static method introduces some sort of "cyclic dependency" is unconvincing to me. [14:57:53] anomie: it's alright, jerkins says no [14:59:42] Yeah, fortuitous merge conflict. Assuming no one else rebases and re-+2s before I have the opportunity to comment there. [15:05:26] Just CR-1 and say you've got comments to come later [16:52:49] Has anyone here seen T221577? DBA folks are worried that it may be some bigger problem, but 80k/day seems like its worth a look reguardless [16:52:49] T221577: Wikimedia\Rdbms\LBFactory::getEmptyTransactionTicket: GeoData\Hooks::doLinksUpdate does not have outer scope - https://phabricator.wikimedia.org/T221577 [17:45:02] AaronSchulz: ^ That looks like something you'd know best about [20:42:44] tgr|away, James_F, please before I get killed by merge conflicts, help me with this: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/506695 [20:44:57] xSavitar: Have C+1'ed, but EditPage is a rather scary area. Should be fine, but I'd like a third opinion. :-) [20:45:20] Sure, thanks a lot! [20:45:24] Why are you going to get killed by merge conflicts? [20:45:43] Reedy: Check the history, I've rebased several times, manually, it's hard ;) [20:45:48] It's not hard [20:45:58] Okay [20:46:11] RELEASE-NOTES in most cases, you just take their chunk, and add yours after it [20:46:16] It's one of the simplest you can do [20:46:55] Yeah, but multiple times gets me freaking out [20:47:05] But yeah, I'll try to cope with it :) [20:47:20] Don't keep rebasing it? [20:47:43] Sounds like a plan, thanks! [20:48:09] Reedy: but it depends the changeset [20:48:47] simple ones like this is fine but for more complicated ones, one needs to be always in sync as the merge conflicts can just pile up and discourage one from continuing the patch after a while [20:48:50] What do you think? [20:48:54] No [20:49:13] there's not usually that much churn in MediaWiki [20:49:37] Unless you're changing stuff in an area that is getting a lot of other changes... Rebases should be mostly trivial [20:50:51] maybe I'm not getting your point entirely, let me share with you something I wanted to work on recently with an idea @Pmiazga gave [20:51:06] And, yes you're right about areas with much change [20:52:46] * xSavitar sighs, waiting for gerrit to load :( [20:55:00] Reedy: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/465288 & https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Echo/+/465562 [20:55:03] :D [20:55:37] I wanted to pick those up after https://phabricator.wikimedia.org/T206728#5095892 [20:55:51] But when I did a pull and rebase, oh no, I just gave up [21:16:56] xSavitar: a merge conflicted core patch should not have any effect on Echo patches unless they depend on it explicitly (via Depends-On) [21:18:53] Yeah, I'm not saying in anyway that all what Reedy said above is wrong [21:18:58] I actually concur with him [21:19:11] I'm just saying that rebasing every time hard on me ;) [21:19:18] But Reedy says it's not hard, so I believe [21:20:36] tgr: Now for example, those 2 patches I linked to a far behind and a lot of changes have been done, trying to fix the merge conflicts now in that patch is so damn hard [21:20:44] *are [21:20:51] time consuming too [21:21:10] I'll need to settle and fix those 2 patches at least to get them to current master [21:21:23] As I've said before... Try cherry picking onto master, rather than rebasing [21:22:15] Reedy: thank you so much, noted! [21:24:02] xSavitar: Though, I'm sure I saw MaxSem try that on a patch recently and give up because it didn't work (the class_alias in PSR-4) [21:24:40] ok [21:24:56] Alternatively, just fix teh tests to not be teh suxx :P [21:25:23] Heh [21:25:28] Have fun with that :P [21:26:41] https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/502278/14/includes/menu/Group.php [21:26:53] I note they use two strings, not any ::class or anything [21:27:17] git checkout master, git pull, git review -x is what I tend to do [21:27:39] git review -x? [21:27:40] * Reedy looks [21:27:55] Ahh [21:28:01] it's git-review's version of cherry-pick [21:28:06] That's nicer than copying the links from gerrit [21:28:17] I'll try and remember that one :) [21:29:20] you may like https://github.com/openstack/gertty :) [21:29:45] and -X is the same but adds the "cherry-picked from..." line to the commit message, helpful when the GUI cherry-picking fails [21:35:38] xSavitar: Just tried... It does look like there are quite a few conflicts, but it's not too bad... [21:36:15] tgr GUI cherry-picking fails is a thing of the past :) [21:36:40] But if they're full of conflicts, it's not so helpful [21:36:48] jgit not sucking would be a bigger win [21:36:48] Reedy: Okay, sure. I'll try to pick that up again once I have some time for it :), thanks [21:37:13] yeh, though you will know which files have conflicts though :) [21:37:25] tgr: Thanks for that command :) [21:37:55] xSavitar: Though, do remember sometimes it might be easier to just give up with a patch, and start again [21:38:04] :D [21:38:19] Or maybe in this case... Just revert all the conflicted files (out of gerrit), and then cherry pick... [21:38:32] And then just re-do the changes you need to make after you've put it on master [21:38:44] There's numerous strategies and workarounds [21:39:07] o/