| [20:00:27] | <paul121[m]> | there will be more code :P |
| [20:00:40] | <paul121[m]> | * be more *clean* code :P |
| [04:42:33] | * calbasi[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:34] | * dazinism[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:37] | * ChinchillaWashin has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:45] | * spitz234[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * scrdcow[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * evered[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * r3c4ll[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * Mo[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * paul121[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * FarmerEd[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * gbathree[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * jgaehring[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * mstenta[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * RafatKhashan[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * ander[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * GudjonEinarMagnu has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * perfectinfoseeke has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * symbioquine[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:46] | * ionitatelecom[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * IyarkaiTechLab[m has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * davd[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * sgoodall[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * RogerioMbuli[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * MarcosCarballal[ has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * FeiWang[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * margeo[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * donblair[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * MattFletcher[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * ThimmZwiener[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * munjoma[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * nickhudson[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * gretel[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * kunigunde[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * Adam[m]12 has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * runfastthinkslow has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * farmtech[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * toino[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * thattechguy[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * nzsnowman[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * steinfarm[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * ludwa6[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * testuser769[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * monkeyflowerfarm has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * sandg100[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * oliverp44[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * JustGav[m]1 has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * elpronto[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * Noaht[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * FreshiesFarmsLLC has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * UgeshB[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * m035[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * TheSlurpee[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * thattechguy99[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * mindcls[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * GuilhermePerotta has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * botlfarm[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * KarsonWynne[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * CarlosAlberto[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * hra38192639[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * iuresearcherpw[m has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * olaf[m]1 has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * raul[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * aislinnpearson[m has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * JanSonntag[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * ZaneBelkhayat[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * tool172[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:47] | * ggcc18[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * goldi[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * harry[m]1 has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * qoyyuum[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * shane_aldrich[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * justgav[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * eddieironsmith[m has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * JustinCampbell[m has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * SpencerOnazi[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * OmkarEkbote[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * Anonymous[m]12 has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * frederike[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * AllanMacGregor[m has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * matrixtrix[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * EvanKelley[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * dzfarmer[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * AnasHaddad[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * postmanpat[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * petednz[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * and712[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * gunter[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * phantomse[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * skipper_is[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * ChristophWolfes[ has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * courtneylking[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * BrandonSmith[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * leogaggl[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * leku[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:42:48] | * GerardoLisboa[m] has quit (Quit: Bridge terminating on SIGTERM) |
| [04:47:17] | * calbasi[m] has joined #farmos |
| [05:04:45] | * spitz234[m] has joined #farmos |
| [05:04:45] | * dazinism[m] has joined #farmos |
| [05:04:45] | * ChinchillaWashin has joined #farmos |
| [05:04:59] | * FarmerEd[m] has joined #farmos |
| [05:04:59] | * RafatKhashan[m] has joined #farmos |
| [05:04:59] | * gbathree[m] has joined #farmos |
| [05:04:59] | * GudjonEinarMagnu has joined #farmos |
| [05:04:59] | * r3c4ll[m] has joined #farmos |
| [05:04:59] | * evered[m] has joined #farmos |
| [05:04:59] | * perfectinfoseeke has joined #farmos |
| [05:04:59] | * Mo[m] has joined #farmos |
| [05:04:59] | * symbioquine[m] has joined #farmos |
| [05:04:59] | * ionitatelecom[m] has joined #farmos |
| [05:04:59] | * nzsnowman[m] has joined #farmos |
| [05:04:59] | * ander[m] has joined #farmos |
| [05:04:59] | * scrdcow[m] has joined #farmos |
| [05:05:00] | * paul121[m] has joined #farmos |
| [05:05:00] | * mstenta[m] has joined #farmos |
| [05:05:00] | * jgaehring[m] has joined #farmos |
| [05:20:22] | * lordeddi[m] has joined #farmos |
| [11:08:28] | <mstenta[m]> | paul121: curious what you think about this change https://github.com/mstenta/farmOS/commit/cc4006aecfcbfffd6161342d034d1b3... |
| [11:09:00] | <mstenta[m]> | One of the key lines/changes is this one: https://github.com/mstenta/farmOS/commit/cc4006aecfcbfffd6161342d034d1b3... |
| [11:09:30] | <mstenta[m]> | - $event = new MapRenderEvent($map, $element); |
| [11:09:30] | <mstenta[m]> | + $event = new MapRenderEvent($map, $element, $entity_type_manager); |
| [11:10:26] | <mstenta[m]> | It feels a bit weird to me that we have to pass in $entity_type_manager here, since the only place it gets used is in `MapRenderEvent::addBehavior()` |
| [11:10:27] | <mstenta[m]> | for loading the behavior by name |
| [11:10:41] | <mstenta[m]> | the alternative would me to change all of our `addBehavior()` calls to pass a loaded behavior entity instead of just a name |
| [11:10:53] | <mstenta[m]> | and leave it to implementors to load the entity |
| [11:11:24] | <mstenta[m]> | that feels a bit cleaner to me... but less convenient... so i'm on the fence |
| [11:12:48] | <mstenta[m]> | this is a very minor thing... i just have decision fatigue i think - curious to get your opinion |
| [11:14:03] | <mstenta[m]> | i looked for comparable cases in core, and in all of those they pass the dependency into the constructor |
| [11:14:19] | <mstenta[m]> | so if that's easiest, it's already done (in this commit) so i'd be fine with just leaving that |
| [11:15:12] | <paul121[m]> | I actually just saw something related in the Gin source code yesterday... I think we could use `Drupal::classResolver()` |
| [11:15:46] | <paul121[m]> | Gin uses this in a few places to instantiate some of the helper classes it has created (classes that are not plugins, forms, controllers, etc) |
| [11:15:53] | <paul121[m]> | https://api.drupal.org/api/drupal/core%21lib%21Drupal.php/function/Drupa...\ |
| [11:16:05] | <paul121[m]> | https://git.drupalcode.org/project/gin/-/blob/8.x-3.x/includes/form.them... |
| [11:16:18] | <mstenta[m]> | ok... |
| [11:16:26] | <mstenta[m]> | so this would allow us to use `ContainerInjectionInterface`? |
| [11:16:37] | <mstenta[m]> | and would replace `new MapRenderEvent(...)`? |
| [11:16:41] | <paul121[m]> | * This line: https://git.drupalcode.org/project/gin/-/blob/8.x-3.x/includes/form.them... |
| [11:16:41] | <paul121[m]> | Instantiates this class: https://git.drupalcode.org/project/gin/-/blob/8.x-3.x/src/GinContentForm... |
| [11:16:46] | <paul121[m]> | yep! |
| [11:17:01] | <paul121[m]> | MapRenderEvent would get `ContainerInjectionInterface` |
| [11:17:29] | <mstenta[m]> | Interesting - so the entity type manager dependency would be automatically added? What about the other two arguments? `$map` and `$element`? |
| [11:18:18] | <paul121[m]> | Hmm good question. I'd assume you can pass arguments in there |
| [11:18:50] | <mstenta[m]> | Yea that was the one thing I was wondering... and why this felt a little "unclean"... it's mixing dependeny injection and regular args in the constructor |
| [11:19:24] | <mstenta[m]> | Hence why I looked to examples in core... and they use `new [Name]Event()` and just pass everything in manually |
| [11:21:20] | <paul121[m]> | mstenta[m]: oh yeah? huh. Maybe that is the best then |
| [11:21:49] | <mstenta[m]> | It's pretty simple |
| [11:22:14] | <mstenta[m]> | Good news! PHPStan tests are passing now! https://github.com/mstenta/farmOS/actions/runs/3046205634 |
| [11:22:33] | <mstenta[m]> | Just need to see if the other tests area passing - or if I broke anything in the process... 😅 |
| [11:23:05] | <mstenta[m]> | Doing some manual testing right now too... |
| [11:23:14] | <mstenta[m]> | Also, I think this means we're ready for D10! |
| [11:23:31] | <mstenta[m]> | (Or rather... the farmOS code itself is... maybe not all our dependencies yet) |
| [11:33:23] | <paul121[m]> | <mstenta[m]> "Yea that was the one thing I was..." <- maybe a better way would be to have setter methods for the "args" (map and element) after the event object is created. Then use ContainerInjectionInterface for the rest? I'm surprised there isn't an easier way for events to use dependencies |
| [11:33:53] | <mstenta[m]> | yea that thought came up too |
| [11:33:58] | <paul121[m]> | This might be something that should be "internal"... seems prone to breaking changes if we ever need additional dependencies :-) |
| [11:34:08] | <mstenta[m]> | right |
| [11:34:29] | <mstenta[m]> | yea TIL about the `@internal` flag |
| [11:34:47] | <mstenta[m]> | we might want to add that to more of our classes as a precaution |
| [11:35:15] | <mstenta[m]> | PHPStan complained about three core `@internal` classes we are extending... had to add ignore flags for them |
| [11:39:20] | <mstenta[m]> | Tests are passing! Woo! https://github.com/mstenta/farmOS/actions/runs/3046205634 |
| [11:45:02] | <paul121[m]> | does PHPStan have a config file similar to phpcs? |
| [11:46:20] | <mstenta[m]> | yea |
| [11:47:01] | <mstenta[m]> | https://github.com/mstenta/farmOS/blob/cc4006aecfcbfffd6161342d034d1b38f... |
| [11:47:45] | <mstenta[m]> | Opened a merge request for review: https://www.drupal.org/project/farm/issues/3309234 |
| [11:54:29] | <paul121[m]> | were there any common "patterns" we use that we should change? |
| [11:54:36] | <paul121[m]> | lots of commits... :D just looking for the tldr haha |
| [11:54:53] | <mstenta[m]> | yea i know... lots of commits... maybe we could take a look together on the dev call |
| [11:54:57] | <paul121[m]> | I see `!empty()` -> `!is_null()` |
| [11:54:59] | <mstenta[m]> | not really... most were small things |
| [11:55:04] | <mstenta[m]> | yes i was going to mention that one |
| [11:55:39] | <mstenta[m]> | `is_null` is just a bit more specific, and PHPStan likes that, where possible |
| [11:55:49] | <mstenta[m]> | the cases where it changed in that commit are mostly where we are loading entities |
| [11:55:59] | <mstenta[m]> | and entity storage returns either en entity interface object or `null` |
| [11:57:28] | <paul121[m]> | > Use itamair/geophp instead of phayes/geophp |
| [11:57:28] | <paul121[m]> | oh I see!!! I thought I had seen two geophp dependencies showing up in the composer log.... but I didn't want to dig in.... |
| [11:57:38] | <paul121[m]> | so drupal/geofield changed their dependency |
| [11:58:15] | <mstenta[m]> | yea! seems like it was forked in order to get PHP 8 support merged in with an official release |
| [11:58:21] | <mstenta[m]> | since the original repo is all but abandoned |
| [11:58:32] | <mstenta[m]> | and yea, we basically had the library in two places inside `vendor` |
| [14:34:17] | * jonasbits has joined #farmos |
| [18:43:18] | * farmBOT has joined #farmos |