[00:40:49] Complex or simple fix? [00:52:25] Not sure, i would assume if it was simple it might have been fixed by now [00:53:55] Agreed [00:54:06] But also this is software development [00:54:09] cant assume shit [02:32:14] [1/3] You guys probably know all this, but thought I'd share it anyway. In case you didn't know. [02:32:14] [2/3] Tools collection: [02:32:15] [3/3] https://wiki.gamedetectives.net/index.php?title=ARG_Toolbox [03:30:35] https://github.com/miraheze/ImportDump/pull/66 [03:32:51] [1/2] `212 | WARNING | Line exceeds 120 characters; contains 121 [03:32:51] [2/2] | | characters (Generic.Files.LineLength.TooLong)` REALLY? 1 character more lol [03:33:23] [1/5] happens on 2 lines actually lol [03:33:23] [2/5] ` 212 | WARNING | Line exceeds 120 characters; contains 121 [03:33:24] [3/5] | | characters (Generic.Files.LineLength.TooLong) [03:33:24] [4/5] 742 | WARNING | Line exceeds 120 characters; contains 121 [03:33:24] [5/5] | | characters (Generic.Files.LineLength.TooLong)` another is 123 [03:37:21] Sorry my terminal is only 120 characters exactly [09:06:59] <.labster> [1/2] I don't understand a lot of this PR, but this change worries me, adding a value in the middle of an enum [09:06:59] <.labster> [2/2] https://cdn.discordapp.com/attachments/1006789349498699827/1208701184576069692/image.png?ex=65e43db2&is=65d1c8b2&hm=bf519dd2cd7e5b23dfd1f436e6f5f81937083e72e3a05ee7d165243e160a5cfd& [09:26:27] Why are we using ENUMs? [09:33:05] <.labster> Don't ask me, I didn't write it. [09:34:03] <.labster> Presumably because someone thought saving 0.2µs compared to a text index was worth the maintenance cost [09:36:57] WMF deployed code shouldn't use them [09:37:02] See the very old https://phabricator.wikimedia.org/T119173 [09:37:14] And it would be nice to have consistent code standards [10:47:43] good thing this ain't WMF code [10:49:05] 😉 [10:54:43] @cosmicalpha found a nitpick in the PR, left a reviewe [10:54:56] *mistake [10:55:02] not really a nitpick [11:23:19] The reasoning behind it is valid [11:23:23] For once [11:30:40] Nevermind! https://www.php.net/manual/en/language.references.pass.php [11:31:27] can't say I really like that, but it is what it is [16:23:21] [1/2] hello, how can I insert a YouTube video into the gallery? [16:23:21] [2/2] https://cdn.discordapp.com/attachments/1006789349498699827/1208810999717826560/fab77baa748e8888.png?ex=65e4a3f8&is=65d22ef8&hm=2d12f5731b04021a8c4202641a6f7be0690e7a6273cd0f039366138197f277cb& [16:30:31] You need an extension for that [16:31:12] Your wiki.miraheze.org/wiki/Special:ManageWiki/extensions#mw-section-parserhooks [16:31:37] [1/2] or search YouTube in the search bar of [16:31:38] [2/2] Your wiki.miraheze.org/wiki/Special:ManageWiki/extensions [16:41:48] [1/2] you can't insert embedded videos in gallery, you can only add them in a section separately [16:41:48] [2/2] YouTube extension is enough if it's just Youtube [16:42:24] also please ask support questions in #general or #support [17:32:09] Ok, sorry [19:06:05] I'll probably phase out enum eventually but for now just left it how it was originally done, which I probably shouldn't have to begin with... [21:05:03] @bluemoon0332 I tested ImportDump and it seems to work. Do you want to do another review before I add more complexity with notifications or wait till I finish completely? [21:06:55] <.labster> Did you change the enum thing? Enums are append-only. [21:07:33] <.labster> If you put a value in the middle it changes the meaning of previous rows [21:07:52] wat [21:08:06] i don’t entirely understand but that seems very goofy ah silly [21:08:13] <.labster> Or has this not been deployed yet so there’s no data? [21:08:40] Current version of import dump uses enums [21:08:45] So ye it would cause data loss [21:08:53] @cosmicalpha [21:09:04] ENUMs are a mess for many reasons [21:09:11] I have no idea why we are using them [21:09:26] I changed it on beta and it worked fine without loss [21:09:59] But I plan to get rid of ENUM completely anyway [21:12:21] :pepenotes: [21:12:26] <.labster> It’s possible it will be fine because of how PHP executes the statements. As long as you don’t recreate the table and put old data into it it might be fine [21:13:55] [1/5] I just ran [21:13:55] [2/5] ```sql [21:13:55] [3/5] ALTER TABLE importdump_requests [21:13:56] [4/5] MODIFY request_status ENUM('complete', 'declined', 'failed', 'inprogress', 'pending') NOT NULL; [21:13:56] [5/5] ``` [21:13:59] got it- side note, which software installs extension repos/actually used the mediawiki-repos repository [21:14:10] Puppet [21:14:31] It’s always back to puppet [21:14:45] Well technically puppet installs the staging version [21:15:01] Which isn't technically meant to be a usable mediawiki install [21:15:05] But kinda is [21:15:18] Then deploy-tool deploys it [21:15:29] It also auto-deploys MirahezeMagic updates [21:16:12] <.labster> Enums are a good idea in principle, mapping common values to integers for less storage and value validation. But the costs of changing the schema to add new values is high, and with modern (>2000ad) storage and processors, the speed and storage are less costly than programmer time spent on Enums [21:16:31] Tbh though it's probably the most complex and messy code I've ever made with loops in loops, etc... for mediawiki-repos, but functional wise I like it better than submodules [21:16:54] Ah yeah it didn't seem to hard for me this time though but maybe should just do away with them. [21:17:24] Enums not in MariaDB are [21:17:30] The concept is great [21:17:36] The implementation is garbage [21:18:01] Got it [21:18:17] mostly but thats the case with most puppet related stuff [21:18:34] <.labster> If you really want value validation just use a foreign key [21:19:36] It wasn't about the validation as much as it looked nice to have a place where you can find all the possible statuses, but thats the reason I now added a ImportDumpStatus interface to replace the original reason I chose ENUMs. [21:20:40] Now I'm just worried about changing them if it'd cause issues or not... [21:21:02] *changing off them [21:23:12] <.labster> Even my worst schema migrations only had three steps tops [21:23:28] Although tbf, the likelihood of yet another status type later on is very low [21:23:54] <.labster> Famous last words [21:24:04] Incredibly so [21:24:18] It’ll probably be fine™️ [21:30:35] <.labster> [1/6] 1. I am learning databases [21:30:35] <.labster> [2/6] 2. What is this ENUM type? That sounds nice [21:30:36] <.labster> [3/6] 3. This is a perfect use case for trying ENUM [21:30:36] <.labster> [4/6] 4. Changing the schema to add a value was harder than I thought, but this will be the last change [21:30:36] <.labster> [5/6] 5. Arrgh [21:30:37] <.labster> [6/6] 6. Fine, alter table varchar it is [21:31:35] <.labster> I think all backend programmers go through this cycle [21:37:27] Lol yeah I do regret using it but there was a discussion about it initially also and no one was truly against using it so it ended up being used... [23:56:38] I think I use integers and then infer their meaning as a convention [23:56:44] Which incidentally is how MediaWiki does namespaces [23:59:32] :ThinkerMH: