[14:34:39] kindrobot: Happy new year :). Thanks for reminding me the ticket (indirectly with the `recheck`). I am answering to Dani remark [14:39:02] Happy new florent! Thanks. Something might be up with our CI. I can't see the log of the last run (hence the recheck) and it doesn't seem like the recheck did anything... [14:40:03] Oh, nevermind, it went. :) [14:41:25] Is the failure (https://integration.wikimedia.org/ci/job/mwgate-node14-docker/98274/console) due to the tests to fix it being in the other patch? [14:42:16] Yes! [14:42:25] That's because of the tests being in a separated commits [14:42:52] As the patch for the tests sound to me risky and heavy, I wanted to split the patch in two [14:43:10] Maybe that's not a correct move while using Gerrit [14:44:47] The two patches related to the same issues are: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/864782 and https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/864783 [14:44:58] The tests in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/864783 passes FWIW [14:44:59] S [14:45:14] Maybe I should consider merging the two patches into one? [14:47:58] In this case, we should be able to merge the tests first and then rebase it onto the implementation patch. Going forward, it would be easier if related tests are included in the same patch. [14:48:50] Yeah, I cannot go against the philosophy of Gerrit =/ [14:50:11] (I like having many commits in the same Merge Requets / Pull Request on other platforms like Gitlab / Github) [14:51:05] Yeah, I know what you mean. [14:51:29] Good news is that we're slowly transitioning to GitLab. [14:51:42] Hopefully this year is the year. ;) [14:51:53] Wow, it would be awesome 🎉 [14:53:13] Would you mind responding to this comment, so that I can merge your patch in? https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/864783/15/package.json [14:55:37] Ahah, what do you think if I rather take into account the remark, close the ticket and merge the patch into 864782 (the code of the implementation)? So it would be less messy ^^ [14:56:59] Because as I understand, if you merge 864783, the patch would be integrated into master? [14:57:16] Yup [14:57:38] Which may not make sense with the implementation it's testing still being in review. I see what you mean. [14:57:54] Yeah ^^' [14:59:02] Let me just check and answer this comment from Dani: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/864782/comment/859f3c7c_26bed09b/ [14:59:02] And I'll do that [14:59:28] But I think we'll still want to know what necessitated the upgrade/addition of those patches, preferably in the commit/patch message. That helps us down the line if we run into problems with one of those patches and are tempted to roll them back. [14:59:38] *of the packages [15:00:17] o_o; sorry conflated "patches" and "packages" [15:00:23] Sure! You want me to explain here? (I'll explain that in the commit message anyways) [15:00:32] Sure, that'd be great. :) [15:01:51] That's because there has been a split between jest-environment-jsdom and jest since version 29.3.0 (or so, I can find some ticket explaining that) [15:02:17] before that, jest-environment-jsdom was native in jest [15:06:38] version 28.0.0 sorry, as stated here: https://jestjs.io/blog/2022/04/25/jest-28#jsdom-19 [15:09:43] Also I upgraded @vue/vue3-jest in order to benefit from a new feature, the .withImplementation() method: https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/WikiLambda/+/a79be8e01b6ff2691ad37d806ade9097b3583e5f/tests/jest/components/ZImplementation.test.js#80 [15:09:58] Hence all these upgrades [15:15:22] I see. Did you need some newer functionality in jest-environment-jsdom? [15:21:53] Not from jest-environment-jsdom, but we already used it without knowing until it was removed from the core of jest [15:24:01] Ah, ok. What I'm trying to get at is what prompted updating _any_ of the packages? Was it just to stay current or did you need a newer version of something for some reason? [15:24:37] Ah! :) [15:25:04] The reason is the new withImplementation() method, IIRC [15:25:51] Oh, OK. Make sure to mention that along with the jest-environment-jsdom split. That'll answer Ed's questions. [15:25:57] Then, it led me to update both vue3-jest and jest [15:26:11] Sure! :) [15:26:18] Thanks. :D [15:26:23] I'll do my best ^^' [15:26:31] You're welcome! [16:05:27] kindrobot: Done: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikiLambda/+/864783 [16:09:03] Do you want me to merge that one and then you're going to rebase 864782? [16:10:10] I'd rather do the squash into 864782, but I wanted to know whether the commit message was clear enough [16:10:45] Looks good to me! [16:19:41] Done. Hope all this mess did not bother much ^^'