Meeting will happen in #d10readiness on drupal.slack.com.
Hello and welcome to this Drupal 10 readiness meeting!
This meeting:
➤ Is for core and contributed project developers as well as people who have integrations and services related to core. Site developers who want to stay in the know to keep up-to-date for the easiest Drupal 10 upgrade of their sites are also welcome.
➤ Now happens every Monday at 18:00 UTC.
➤ Is done over chat.
➤ Happens in threads, which you can follow to be notified of new replies even if you don’t comment in the thread. You may also join the meeting later and participate asynchronously!
➤ Has a public agenda anyone can add to: `https://www.drupal.org/project/drupal/issues/3292926`
➤ *Transcript will be exported and posted* to the agenda issue. For anonymous comments, start with a :bust_in_silhouette: emoji. To take a comment or thread off the record, start with a :no_entry_sign: emoji.| mglaman | :wave: Matt, maintainer of phpstan-drupal and contributor to drupal-rector |
| Spokje | Spokje, dedicated deprecator (has taken his rage-lowering pills) (edited) |
| xjm | :wave: also medicated :slightly_smiling_face: |
| Björn Brala (bbrala) | Björn, started because of just getting some small fixes in to the bot. Ended up the new expert :') |
| Björn Brala (bbrala) | I wouldv'e helped a bit more @Spokje in your quest. But i have only so much time to spend. |
| Spokje | No worries, you're now an "expert" in your own field and that means you've got no time left. (BTW that's exactly how I rolled into my "specialism")(Notice the (now not so) subtle reference to Sisyphus' rolling stone there... :innocent:) |
| dww | Derek @ TEN7. Wishing I had something witty to say about Quick Edit and our collective mental health. :stuck_out_tongue_winking_eye: |
| Gábor Hojtsy (he/him) | Gábor, core-ordinator |
| shaal | Ofer Shaal :blob_wave: |
| baddysonja | Baddy Sonja, catching up on the next day and see how we can best prepare for Drupal 10 and help out. |
| Kristen Pol (she/her) | Kristen, catching up and wishing everyone healthy vibes :blue_heart: |
| mglaman | Are we collecting/aggregating feedback? I’m noticing weird problems: DrupalCI not failing on PHPUnit deprecations (but locally I see them.) Really off Rector patches (missing file constant etc fixes). I need to write these up myself, but I know we’re getting feedback but I don’t know where it’s going. The Rector queue doesn’t seem like the right place. (edited) |
| xjm | Spokje's thread above |
| xjm | 10.1.x branch info (edited) |
| Spokje | People with an actual (JavaScript-minded) brain needed to get the last (mwoahaha!) blocking code issue for Quick Edit removal across the line, because I fail spectacularly at it. |
| Kristen Pol (she/her) | Given the various recent "issues" reported for Project Update Bot patches, should we be holding off on encouraging porting and events around porting? |
| xjm | :plus_one: we can see if my team should help re: QE removal |
| catch | Contrib testing against 9.5.x for modules being deprecated/removed in 9.5 |
| catch | @xjm new ckeditor5/qe blocker just dropped |
| mglaman | re: @Kristen Pol (she/her), there’s a lot of work for jquery.once to be done, so I wouldn’t hold off. Lots of manual work |
| Gábor Hojtsy (he/him) | Folks, if you can stop responding to thread proposals here, trying to figure out which are thread proposals would be easier :stuck_out_tongue: |
| xjm | (soz) |
| Spokje | Input wanted/needed for deprecation/removal Core modules new/extended policies. |
| xjm | Discussion of project update bot and jQuery package deprecations /cc @mglaman |
| Gábor Hojtsy (he/him) | @catch was “new ckeditor5/qe blocker just dropped” supposed to be a new thread suggestion? (edited) |
| catch | @Gábor Hojtsy (he/him) it was until I saw it had already been raised by @Spokje |
| Gábor Hojtsy (he/him) | Raised by @mglaman |
| Gábor Hojtsy (he/him) | is there an existing issue? |
| mglaman | I have no idea. But when working on linkit I got a whole bunch of deprecations locally with tests but not on DrupalCI |
| mglaman | oh duh.. that issue.. |
| mglaman | #3292870: Drupal 10 compatibility for 8.x-5.x branch of Linkit no deprecation failures came back from drupalci |
| berdir | Isn't that the normal behaviour unless you customize drupalci.yml? |
| berdir | See the file/change in the entity browser issue for example |
| mglaman | I thought it was always on, unless you opted out |
| berdir | No |
| mglaman | oh really?!?! crap |
| berdir | And I'd strongly recommend against having it on by default, or a new minor version or a dependy can break your tests |
| mglaman | So part of the porting runbook is including a drupalci to turn it on, but don’t commit (cc @Kristen Pol (she/her). Otherwise you don’t know how well off you are |
| berdir | I'd love to see that as a build configuration so you can have a weekly test with deprecations to track that for example |
| mglaman | which that can be replaced with drupalci as a “allowed-fail” job within the workflow |
| berdir | See #3292775: Drupal 10 compatibility for an example |
| berdir | Of course, if you have no dependencies or they are all compatible with d10 you can just run directly against that |
| Kristen Pol (she/her) | Okay, not I'm understanding this... here's my attempt:1) drupalci is not enabled by default2) If drupalci is setup, then test deprecations will be caught in CI3) If drupalci is not setup, the bot won't find test deprecations4) If drupalci is not setup, we have to check test deprecations ourselves to see if they are any and fix them with rector (or manually) and update the readiness patch5) For "the porting runbook is including a drupalci to turn it on, but don’t commit", do you mean to add the drupalci yml file in the readiness patch but don't commit it to the project code? (edited) |
| berdir | default drupalci configuration suppresses deprecation messages, so you need to set that flag in it to not suppress them. And 5 is correct, yes |
| Kristen Pol (she/her) | @berdir thanks for the clarification :) |
| Kristen Pol (she/her) | Rewritten... is this closer?1) drupalci deprecation messages are disabled/suppressed by default2) If drupalci deprecation messages are enabled, then test deprecations will be caught in CI3) If drupalci deprecation messages are disabled, the bot won't find test deprecations (IS THIS CORRECT?)4) If drupalci deprecation messages are disabled, we have to check test deprecations ourselves to see if they are any and fix them with rector (or manually) and update the readiness patchOR5) If disabled, we need to enable deprecation messages in the drupalci yml file within the readiness patch but don't commit it to the project code so we can see the failures in the readiness issue (edited) |
| mglaman | #3 is correct, we can’t detect deprecations on DI without them being enabled. Only “run on d10 and see how it crashes” |
| Kristen Pol (she/her) | Okay, so all 5 points are clear/correct? |
| mglaman | correct |
| berdir | yeah, but I'd add that if you do 5 (let the testbot report deprecations), then you don't really need to do 4 (test it yourself). Plus, if you can, do D10 test runs as well. not everything will be caught be deprecation messages. but that requires that all dependencies (including require-dev) modules have at least a development snapshot (committed) compatible with d10 already |
| mglaman | cc @japerry that doc we worked on, does it cover stuff on this 1-5 @Kristen Pol (she/her) is mentioning? maybe we can share? |
| japerry | I don't see why we couldn't share that doc |
| japerry | its covers public module processes |
| mglaman | can you link it here? Maybe it’ll be useful |
| japerry | https://docs.google.com/document/d/1xJzD_sIWn8vbnLtjlr4ITz5rbv8KJpK5ydEN... |
| japerry | and no it doesn't talk about changing drupalci .yml |
| japerry | so we'd need to update it to include those steps |
| Kristen Pol (she/her) | Looking, thanks |
| Kristen Pol (she/her) | @berdir I just put "OR" between 4 and 5 to cover your point :slightly_smiling_face: |
| Gábor Hojtsy (he/him) | Raised by @mglaman, with example: (missing file constant etc fixes) |
| Gábor Hojtsy (he/him) | I think @berdir has also noted where only one of his test files was touched by rector while various other files used the same patterns |
| mglaman | is this due to lagging drupal-rector version on update bot? (edited) |
| mglaman | I didn’t compare what was missing vs what rules were added to drupal-rector |
| mglaman | maybe @Björn Brala (bbrala) knows, as I know he was working into fixing the behind version |
| Gábor Hojtsy (he/him) | I think some of this has to do with the rector setup being a bit outdated when we initially ran it in https://dispatcher.drupalci.org/job/project_analysis_d10/127/ |
| Gábor Hojtsy (he/him) | the most recent run at https://dispatcher.drupalci.org/job/project_analysis_d10/136/ has an updated rector |
| Björn Brala (bbrala) | 0.12.4 last run, that was yesterday though |
| Björn Brala (bbrala) | If you want to compare output. |
| Björn Brala (bbrala) | https://github.com/bbrala/drupal10-readiness/tree/127-1304-compare |
| Björn Brala (bbrala) | This has the different runs and the results in it |
| Björn Brala (bbrala) | I have not had the time to check out the specific results mentioned regarding for example only one setUp method getting updated unfortunately |
| Björn Brala (bbrala) | I do feel lagging rector version was a problem, but if we look at some specific modules and compare patches we should be able to figure out if it is still a problem. |
| mglaman | yes, I haven’t had the time to. My day-to-day work on d10 readiness is ramping up |
| mglaman | so I may have time soon to dig into the “why?!” |
| Björn Brala (bbrala) | Do you have a module where you know this issue still exists? |
| Björn Brala (bbrala) | Could check later tonight |
| mglaman | linkit had the problem, the automated patch only had setUp(): void fixes but nothing else |
| Björn Brala (bbrala) | This is the dfiff between 127 and 136 in regards to the linkit patch: |
| mglaman | that looks :100: |
| mglaman | those are the items I had to fix |
| Björn Brala (bbrala) | Thats encouraging :wink: |
| Björn Brala (bbrala) | That change is pretty much only the upgrade of upgrade_status and drupal-rector |
| Björn Brala (bbrala) | Had some unfortunate few runs where other issues ended up screwing with the end result :disappointed: |
| Björn Brala (bbrala) | It might be wise @Gábor Hojtsy (he/him) to get this put online asap. |
| Björn Brala (bbrala) | And perhaps have our good friend @tedbow update patches soon. |
| Björn Brala (bbrala) | Think there is a lot missed right now in the current post run |
| Gábor Hojtsy (he/him) | @Björn Brala (bbrala) ah beautiful :slightly_smiling_face: contact storage was the one raised by @berdir, can you check that diff too? |
| Björn Brala (bbrala) | yes |
| Björn Brala (bbrala) | Seems rector cannot fix a lot in that module. |
| Björn Brala (bbrala) | contact_storage_export has a patch though |
| Björn Brala (bbrala) | concatact_storage_options_email_recipient also |
| Björn Brala (bbrala) | phpstorm hangs... xD |
| Gábor Hojtsy (he/him) | yeah maybe @berdir fixed a bunch of stuff in the meantime in the base module (edited) |
| Björn Brala (bbrala) | no there are issues left. Just rector doesnt fix them it seems |
| Björn Brala (bbrala) | never mind... phpstorm is just lagging. brb |
| Björn Brala (bbrala) | this is the 136 patch, didnt look in it :wink: |
| Björn Brala (bbrala) | no info.yml change, so appearantly there are issues left. |
| Björn Brala (bbrala) | post rector errors |
| berdir | same as before I think. just one changed file, others tests should have dozens of changes |
| Björn Brala (bbrala) | 38kb post rector o_O |
| Björn Brala (bbrala) | (think phpstorm hates me for this) |
| berdir | understandable |
| Björn Brala (bbrala) | @mglaman do you know if some of these should be fixed in current latest release of rector? |
| Björn Brala (bbrala) | contact_storage.1.x-dev.upgrade_status.post_rector.xml
<?xml version="1.0"?> Click to expand inline (367 lines) |
| mglaman | glancing at above, yes. There are definitely some gaps. But most look covered |
| mglaman | It needs triage. I haven’t had energy to review drupal-rector vs current reports |
| berdir | I don't think it's about specific issues, more about files. it only changes one file, and issues it fixes in there are not fixed in others |
| berdir | maybe it chokes on something in those files? |
| Björn Brala (bbrala) | hmm |
| mglaman | @berdir that was your problem, right? it gave a partial patch. like it crashed and generated a little bit of a diff |
| Björn Brala (bbrala) | This is a stderror on that module: |
| Björn Brala (bbrala) | Fatal error: Uncaught AssertionError: assert($pathValue instanceof Node\Scalar\String_) in /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/palantirnet/drupal-rector/src/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php:74 Stack trace: #0 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/palantirnet/drupal-rector/src/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php(74): assert(false, 'assert($pathVal...') #1 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Rector/AbstractRector.php(243): DrupalRector\Rector\Deprecation\UiHelperTraitDrupalPostFormRector->refactor(Object(PhpParser\Node\Expr\MethodCall)) #2 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(113): Rector\Core\Rector\AbstractRector->enterNode(Object(PhpParser\Node\Expr\MethodCall)) #3 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(196): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Expression)) #4 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(105): PhpParser\NodeTraverser->traverseArray(Array) #5 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(196): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\ClassMethod)) #6 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(105): PhpParser\NodeTraverser->traverseArray(Array) #7 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(196): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Class_)) #8 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(105): PhpParser\NodeTraverser->traverseArray(Array) #9 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(196): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Namespace_)) #10 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(85): PhpParser\NodeTraverser->traverseArray(Array) #11 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/PhpParser/NodeTraverser/RectorNodeTraverser.php(56): PhpParser\NodeTraverser->traverse(Array) #12 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Application/FileProcessor.php(54): Rector\Core\PhpParser\NodeTraverser\RectorNodeTraverser->traverse(Array) #13 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Application/FileProcessor/PhpFileProcessor.php(137): Rector\Core\Application\FileProcessor->refactor(Object(Rector\Core\ValueObject\Application\File), Object(Rector\Core\ValueObject\Configuration)) #14 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Application/FileProcessor/PhpFileProcessor.php(101): Rector\Core\Application\FileProcessor\PhpFileProcessor->refactorNodesWithRectors(Object(Rector\Core\ValueObject\Application\File), Object(Rector\Core\ValueObject\Configuration)) #15 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Application/ApplicationFileProcessor.php(175): Rector\Core\Application\FileProcessor\PhpFileProcessor->process(Object(Rector\Core\ValueObject\Application\File), Object(Rector\Core\ValueObject\Configuration)) #16 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Application/ApplicationFileProcessor.php(148): Rector\Core\Application\ApplicationFileProcessor->processFiles(Array, Object(Rector\Core\ValueObject\Configuration)) #17 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Console/Command/ProcessCommand.php(139): Rector\Core\Application\ApplicationFileProcessor->run(Object(Rector\Core\ValueObject\Configuration), Object(RectorPrefix20220117\Symfony\Component\Console\Input\ArgvInput)) #18 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/symfony/console/Command/Command.php(282): Rector\Core\Console\Command\ProcessCommand->execute(Object(RectorPrefix20220117\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220117\Symfony\Component\Console\Output\ConsoleOutput)) #19 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/symfony/console/Application.php(853): RectorPrefix20220117\Symfony\Component\Console\Command\Command->run(Object(RectorPrefix20220117\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220117\Symfony\Component\Console\Output\ConsoleOutput)) #20 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/symfony/console/Application.php(289): RectorPrefix20220117\Symfony\Component\Console\Application->doRunCommand(Object(Rector\Core\Console\Command\ProcessCommand), Object(RectorPrefix20220117\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220117\Symfony\Component\Console\Output\ConsoleOutput)) #21 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/src/Console/ConsoleApplication.php(67): RectorPrefix20220117\Symfony\Component\Console\Application->doRun(Object(RectorPrefix20220117\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220117\Symfony\Component\Console\Output\ConsoleOutput)) #22 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/symfony/console/Application.php(187): Rector\Core\Console\ConsoleApplication->doRun(Object(RectorPrefix20220117\Symfony\Component\Console\Input\ArgvInput), Object(RectorPrefix20220117\Symfony\Component\Console\Output\ConsoleOutput)) #23 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/bin/rector.php(56): RectorPrefix20220117\Symfony\Component\Console\Application->run() #24 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/bin/rector(5): require_once('/var/lib/drupal...') #25 /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/bin/rector(117): include('/var/lib/drupal...') #26 {main} thrown in /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/palantirnet/drupal-rector/src/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php on line 74 |
| Björn Brala (bbrala) | That does look like a crash :x |
| Björn Brala (bbrala) | contact_storage_export.1.x-dev.rector_stderr
Notice: RectorPrefix20220117\Nette\Utils\Strings::toAscii(): it is recommended to enable PHP extensions 'intl'. in /var/lib/drupalci/workspace/drupal-checkouts/drupalxx/vendor/rector/rector/vendor/nette/utils/src/Utils/Strings.php on line 124 Click to expand inline (32 lines) |
| Björn Brala (bbrala) | same error in contact-storage actually... |
| mglaman | that belongs on drupal-rector queue for sure |
| Björn Brala (bbrala) | i'm gonna do a quick find on that error and see how many times this comes up |
| Björn Brala (bbrala) | brb, cause indexing will take a while |
| Gábor Hojtsy (he/him) | yay, good digging here :heart_eyes: |
| berdir | yeah, expected exactly something like this. looks like the drupalPostForm handling, probably doesn't like a certain dynamic version of that |
| Björn Brala (bbrala) | image.png |
| Björn Brala (bbrala) | seems like it might be reproducable xD |
| Gábor Hojtsy (he/him) | This may not yet be perfect, but 136 still look better than 127 to update the d.o issues @tedbow :slightly_smiling_face: |
| mglaman | awesme, if you can post that to the rector issues.. I’ll try to get on it. I’ve had to take on head coach for my son’s baseball. Games every Wednesday have disrupted my normal dev time for this |
| Björn Brala (bbrala) | There are more errors though. But perhaps those are mostly classload errors |
| Björn Brala (bbrala) | image.png |
| Björn Brala (bbrala) | Would you like an issue for each fatal @mglaman? |
| Björn Brala (bbrala) | Fatal error: Declaration of Drupal\Tests\alt_stream_wrappers\Kernel\AltStreamWrapperTest::setUp() must be compatible with Drupal\KernelTests\Core\File\StreamWrapperTest::setUp(): void in /var/lib/drupalci/workspace/drupal-checkouts/drupalxxx/modules/contrib/alt_stream_wrappers/tests/src/Kernel/AltStreamWrapperTest.php on line 17Another one, this one feels weird though :/ |
| mglaman | I think one for each. And :point_up: is one that’s hard to dance around (it should have had a replacement fixed in the test class shim) |
| Björn Brala (bbrala) | Ok, i'll composer a list of issues with a few modules that show that error. Ok if i just post error + modules or would you prefer a package with the whole debug stuff. Let me make a quick example of how that would look like: |
| Björn Brala (bbrala) | Zip contact-storage-136.zipZip |
| berdir | that example above is because it subclasses a non-base core test class, so the magic that core does doesn't apply there, redis had a similar problem as it also subclasses a bunch of core tests and re-runs them against redis |
| Björn Brala (bbrala) | ok, fair enough. Still feels kinda weird to fatal on that though? |
| mglaman | can’t avoid it, PHP fatals. (edited) |
| Björn Brala (bbrala) | ah |
| Björn Brala (bbrala) | unparsable so cannot work on it. |
| mglaman | the static reflection of phpstan 1.7 should fix that, but I don’t think it does (and has other quirks) |
| Björn Brala (bbrala) | I'll skip those then and skip the php 8 incompatibilities :x |
| Björn Brala (bbrala) | Would sometihng like this then be a missing dependency?Fatal error: During class fetch: Uncaught PHPStan\Broker\ClassAutoloadingException: Class Robo\Task\Archive\loadTasks not found. in phar:///var/lib/drupalci/workspace/drupal-checkouts/drupalxxx/vendor/rector/rector/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/Runtime/RuntimeReflectionProvider.php:185 Stack trace: #0 /var/lib/drupalci/workspace/drupal-checkouts/drupalxxx/themes/contrib/bootstrap_storybook/Commands/bootstrap_storybook/SubThemeCommands.php(26): PHPStan\Reflection\Runtime\RuntimeReflectionProvider->PHPStan\Reflection\Runtime\{closure}('Robo\\Task\\Archi...') #1 /var/lib/drupalci/workspace/drupal-checkouts/drupalxxx/vendor/rector/rector/vendor/composer/ClassLoader.php(571): include('/var/lib/drupal...') |
| Gábor Hojtsy (he/him) | Somehow the missing dependencies / classes not found vanished from the upgrade status results, so it looks like they are now back in rector then :stuck_out_tongue: |
| Björn Brala (bbrala) | xD not as many though it seems |
| Björn Brala (bbrala) | Basically if my above statement is true, there is 2 issues ill openFatal error: Uncaught AssertionError: assert($scope instanceof Scope) in /var/lib/drupalci/workspace/drupal-checkouts/drupalxxx/vendor/palantirnet/drupal-rector/src/Utility/GetDeclaringSourceTrait.php:24andFatal error: Uncaught AssertionError: assert($pathValue instanceof Node\Scalar\String_) in /var/lib/drupalci/workspace/drupal-checkouts/drupalxxx/vendor/palantirnet/drupal-rector/src/Rector/Deprecation/UiHelperTraitDrupalPostFormRector.php:86 |
| mglaman | way to go, Matt, for using assert everywhere. Half joking, otherwise it would have been some other spectacular crash |
| Björn Brala (bbrala) | First issue: #3292939: Fatal error: Uncaught AssertionError: assert($pathValue instanceof Node\Scalar\String_) |
| Björn Brala (bbrala) | I was thinking, im not sure what version of 9.4 core it is running against :confused: |
| Björn Brala (bbrala) | Second issue: #3292943: Fatal error: Uncaught AssertionError: assert($scope instanceof Scope) |
| mglaman | Did the error have any indication as to which file? if not, I can try to replicate in the unit tests |
| Björn Brala (bbrala) | Ah |
| Björn Brala (bbrala) | I could add a few of the xml results, not gonna do it for 100 modules though :wink: |
| mglaman | nah |
| Björn Brala (bbrala) | 15 seconds per module, ill post a few zips |
| Björn Brala (bbrala) | wish i could upload multilpe files in drupal.org :confused: |
| Björn Brala (bbrala) | I've uploaded the results for a few modules at both issues |
| mglaman | thx!!! |
| Björn Brala (bbrala) | all the debugging you could ever wwant i think |
| Kristen Pol (she/her) | Thanks for all the debugging and fixes @Björn Brala (bbrala) and @mglaman!!! |
| Gábor Hojtsy (he/him) | Raised by @xjm, I tried to summarize what is the main message I think. Will update the topic if needed. |
| xjm | Yeah the TLDR is:If you have a new deprecation (except modules)If your issue requires an upgrade path (even small)If your issue is refactoring or other disruptions/risksIf your issue is a major new featureIf your issue requires PHP 8.1 or D10 etc.....Then it can be committed to 10.1.x staring now! |
| xjm | The only restrictions are that we want to limit things that will break cherry-pick especially for JS builds, so JS dep updates must be the same for 10.0.x and 10.1.x, etc. |
| xjm | @Gábor Hojtsy (he/him) I think you can edit your mesasge to say "but changes that might break cherry-pick should be held off" -- some big changes are OK so long as they're not messing with JS build basically. :slightly_smiling_face: |
| Gábor Hojtsy (he/him) | Yeah that is what I meant, but you had better words :slightly_smiling_face: updated, thanks |
| Gábor Hojtsy (he/him) | Raised by @Spokje |
| xjm | Can we link the issue(s) for this and I will add them to the DAT backlog for next sprint? |
| Spokje | I hope some JS-Guru will have a crack at #3292780: Move Quick Edit related Javascript from core/modules/ckeditor5/js/ckeditor5.es6.js::detach() to the Quick Edit module.I also hope the IS in that issue is clear enough to say it all.Tried it myself, but all my approaches failed in different spectacular ways :confused: |
| xjm | Should be able to get some pro frontend JS committer help that way :slightly_smiling_face: |
| xjm | Thanks -- adding it |
| xjm | It might be a week and a half or so until we start on it given other stuff with CKE, but at least it's there so we can try to unblock it when available. |
| Spokje | Well, if I look at the wacky write-up I put here: https://drupal.slack.com/archives/C014CT1CN1M/p1656352055778989, I think there's enough other things to keep us/me/somebody busy before we actually need that code change, or any code change on those deprecation/removal issues :slightly_smiling_face: |
| dww | @Spokje Is this JS failure the thing causing QE contrib to fail spectacularly when testing against 10.0.x? |
| Spokje | @dww I need to look into that, there's actually a $consultancy_day_job that needs my attention first, I hope to get to that late in (my) afternoon. I don't think it has anything to do with the massive fails, something else is fishy, me thinks, |
| Spokje | Hmm, I really don't understand why we can test this MR in the first place, as stated in the issue the replace section for drupal/quickedit is still in 9.5.x and 10.0.x which makes all my feeble attempts to test anything Contrib QE relates getting shot down (#3291700: new subtree split of core Quick Edit into contrib (v2)#comment-14588508)I can think of two options: This has to do something with the fact that when opening that issue drupal/quickedit-quickedit was (still) in place or magical elven dust was sprinkled around.I really hope the latter, but am open for any other explanation. |
| dww | Because when the bot is testing a contrib project, we check it out directly from Git. The bot uses composer for everything else, but the project you’re testing is a direct checkout... |
| Spokje | Hmmm, this does seem to work for an MR but not for a patch then? You did MR: Succes!, I did patch: CLUNK!? |
| Gábor Hojtsy (he/him) | Raised by @catch |
| catch | Currently contrib can test against 9.4 and 10.0 but not 9.5. I'm not sure what makes 9.5 an option but it would be useful for when we're deprecating modules in 9.5 |
| Gábor Hojtsy (he/him) | @mixologic maintains that list I think |
| xjm | I think this is a @mixologic needed topic? |
| Spokje | I even think it's essential. Testing against 9.4.x isn't a real world scenario. Even testing against a moving target that 9.5.x currently is, is much closer to reality IMHO. (edited) |
| catch | Yes especially when ideally the modules are subtree splits from 9.5 |
| mixologic | Oh, we need branch labels for 9.5. I will update those today. |
| mixologic | Updated now. |
| Gábor Hojtsy (he/him) | @Kristen Pol (she/her) asked this given the outstanding work on project update bot |
| mglaman | Now. jquery.once to once and jQuery UI deprecations/contrib modules are a HUGE effort |
| Gábor Hojtsy (he/him) | I think there will almost always be some “if we do this the bot will be much better”, so don’t hold off :slightly_smiling_face: |
| Gábor Hojtsy (he/him) | also hundreds of modules got declared Drupal 10 ready recently, so testing those would also be useful :smile: |
| xjm | @mglaman I am confused about the jQuery UI deprecation stuff here. Most of those were for D9.In D10 we decided to defer them until we had tested a11y_autocomplete, so we are no longer deprecating jQuery UI, Dialog, Autocomplete, or their dependencies. We are using the maintained versions because it got un-EOL.The packages deprecated in D9 remains so. |
| xjm | @mglaman This might be worth its own thread because something is wrong here |
| mglaman | core.libraries.yml still says they’re deprecated, change record says to use contrib module |
| xjm | Suggested a thread ^ |
| Kristen Pol (she/her) | Thanks all!!! |
| Gábor Hojtsy (he/him) | back to this topic, I think humans should be good now :slightly_smiling_face: |
| Gábor Hojtsy (he/him) | we’ll keep posting updates, but by the time you have an event together, we will work out any major problems IMHO |
| Gábor Hojtsy (he/him) | when is the time you plan to have the first/next event? |
| Kristen Pol (she/her) | Sadly haven't gotten much feedback yet: https://twitter.com/kristen_pol/status/1541846347430514689?s=20&t=xybTiT... |
| Kristen Pol (she/her) | I was thinking the first one could be in late July to give us time to find mentors and advertise |
| Gábor Hojtsy (he/him) | Yeah the bot should be very good by then. It's good now and it will only get better :) |
| Kristen Pol (she/her) | Cool! I'm planning on getting more organized next week and may even set up a site to promote it... stay tuned :slightly_smiling_face: |
| Gábor Hojtsy (he/him) | Raised by @Spokje, can you copy the text in here instead of linking for the sake of saving the notes? thanks! |
| xjm | Or maybe an issue? |
| xjm | I'll need time to digest and get caught up on it |
| Gábor Hojtsy (he/him) | I think 6️⃣ may have covered part of it so was hesitant which parts to copy over myself. |
| Spokje | I think we'll go issue? Since a) it's a long text, b) we have a centralized place for comments instead of 2 slack threads (the original one and in here) |
| Spokje | And yes, @catch stole 6️⃣ from that list. :smiling_imp: |
| Spokje | [#3292932]Please give all your feedback in there, not here :) |
| Gábor Hojtsy (he/him) | Raised by @xjm and @mglaman |
| mglaman | core.libraries.yml has various jQuery libraries marked as deprecated. I guess they aren’t exactly deprecated. Change record isn’t updated to reflect this.Happened when updating linkit which uses jQuery UI autocomplete |
| Gábor Hojtsy (he/him) | deprecation status should find this and so the bot should also consider this |
| Gábor Hojtsy (he/him) | not sure how is this related to the bot? |
| xjm | @mglaman So some should be deprecated and some aren't. The ones that were deprecated were deprecated in D9. I'm trying to grok core.libraries.yml atm |
| mglaman | Not related to bot, but just D10 work |
| mglaman | upgrade_status is properly reporting, but I guess there are changes in policy |
| xjm | The comments mention deprecation, but none of the remaining libraries are actually deprecated |
| Gábor Hojtsy (he/him) | 701 deprecated library extensions found in contrib: https://dev.acquia.com/drupal10/deprecation_status/errors?error=%2Adepen... (edited) |
| xjm | Ugh I am looking at 10.0.x, sorry, let's look at the right branch |
| mglaman | So does 9.3.x/9.4.x need a patch release doing what we did w/ PHPunit? Deprecated, but punting 2x versions |
| xjm | Just checked and it looks like we had a few cleanups that missed D9 |
| xjm | autocomplete, dialog, and their dependents (including jquery_ui core) are not deprecated |
| xjm | So I'm trying to understand what the problem is then |
| mglaman | jquery.ui.autocomplete: version: *jquery_ui_version license: *jquery_ui_license ... deprecated: *jquery_ui_unused_deprecated |
| catch | Didn't we deprecate all of the jquery_ui libraries? Autocomplete isn't gone but it's directly included in Drupal autocomplete now. But this is memory not at computer. |
| mglaman | If someone has jquery.ui.autocomplete as a dependnecy, we get a warning |
| catch | Yeah that is correct. |
| mglaman | And the change record says to use the contrib via drupal.autocomplete |
| mglaman | And none of the contrib replacements at D10 ready, so we can’t actually fix this. |
| xjm | OK now I'm confused.... :crystal_ball: Summoning @lauriii... I thought we deferred this to D10 |
| catch | I think the problem is the contrib modules have fallen behind, not the core deprecations. |
| xjm | @lauriii Do we need to undeprecate them or are there equivalent internal libraries? |
| catch | @xjm we directly reference the jQuery UI files in core for stuff we couldn't completely remove but there is no library for them. |
| xjm | @catch But in December I swear we decided to leave autocomplete as-is in D10, and then deprecate it in D10 once ally_autocomplete was tested, without the BC shim wildness |
| catch | #3113400: Deprecate more jQuery UI library definitions |
| xjm | Like we made a choice to defer that work to D10/D11. /cc also @bnjmnm halp :slightly_smiling_face: |
| catch | @xjm the code yes, the library no. |
| catch | It was my issue that did it. |
| catch | The library deprecations in core are correct. |
| catch | The problem is the contrib modules were created circa 9.2 and are out of date. |
| xjm | But I thought we decided not to do contrib modules for the core parts we use yet since we want to test ally_autocomplete against D10 and not deprecate jquery_ui autocomplete until at least 10.1.x for D11 removal |
| xjm | So autocomplete, dialog, and their dependents should not be deprecates |
| xjm | No? |
| catch | No because we include jquery_ui directly in the things that need it. |
| catch | Please read the issue I linked it is all in there. I am in phone walking ATM and about to be driving. |
| xjm | I did but it predates our change of policy for autocomplete by 5 months |
| xjm | Made a note to discuss this with JS committers and FEFMs when they are available earlier in the day |
| catch | Yes but it's what allowed autocomplete to not be an issue. |
| catch | Because core no longer provides jQuery UI in libraries it includes it directly as per that issue. |
| xjm | Autocomplete is something that does have an API and gets interacted with though, as is dialog |
| xjm | And AFAIK jQuery UI APIs for them were exposed |
| catch | Yes but you can't depend on jQuery UI directly. |
| xjm | We un-deprecated jQuery UI in core in December when 1.13.0 came out |
| catch | Because there is no library definition for it. |
| bnjmnm | I'm just gonna say Catch is right here |
| catch | Because we removed the library definitions. |
| xjm | So jQuery UI is undeprecated, but autocomplete and dialog are not? |
| xjm | I am so confused :sob: |
| catch | Autocomplete and dialog are core libraries that directly depend on jQuery UI, not via separate library definitions. Per the issue I linked. |
| xjm | Yes but then 1.13.0 |
| catch | That was forking. Not deprecation.o |
| catch | We unforked. We didn't undeprecate |
| xjm | But we were unforking so shouldn't it be undeprecated?I swear the decision was to stick with D9 autocomplete adn dialog, and then add their replacements as seprate packages -- or even keep jQuery UI for them -- until it was ready |
| catch | No it shouldn't be undeprecated because we still want to move away from it and not expose it as its own ali |
| xjm | Like there were long discussions about how we now have a relationship with them and so maybe can trust people with it for updates |
| xjm | Adn that WP still relied on them and etc. |
| xjm | A related concern now that they are unforked -- for the components we actually removed -- is the maintenance nightmare |
| catch | So if we do that in 10-11 contrib will be unaffected because it will be moving to the contrib module. Unless it is integrating specifically with core autocomplete which is different. |
| bnjmnm | That's why we don't have to get rid of drupal.dialog and drupal.autocomplete using jQuery UI assets. However, they load them directly as opposed to depending on the now-deprecated core libraries |
| xjm | OK, I think I get it, sorta.... but |
| xjm | jQuery UI doesn't package them individually, but the contrib projects do for the libraries. Even leaving aside autocomplete and dialog (which is problematic because two copies of that on a seite would be bad) We are violating sec team policy by having asset code committed to the repos. |
| xjm | And they're mostly not on 1.13 except the one that had an SA |
| xjm | Which was fine when they were dead forks, but now that it's maintained again, they need maintenance and maybe an automated process. |
| xjm | like since it's contrib we could give them a build script for the library instead of the library source, and tell them to run it on install. (contrib can require node if needed.) |
| xjm | I think that would be fine for the libraries core doesn't use, but worried about the "two autocompletes" problem if contrib tries to use a jQuery UI autocomplete package. |
| xjm | So maybe we need two issues:Rebuild non-core-used jQuery UI contrib projects to contain a build script and instructions to run it, rather than the asset version committed to the rep.Discuss whether same is appropriate for the stuff still used internally by core.(edited) |
| catch | The contrib jquery_ui projects in those cases could probably reference the core files directly if that's a problem. Until they're physically removed. |
| xjm | @catch For #2 ? |
| xjm | I'll see if I can get Lauri and Théo to help on this -- thanks @catch and @bnjmnm for helping me understand the current situation |
| catch | Yes for #2 |
| catch | Back on computer, in response to https://drupal.slack.com/archives/C014CT1CN1M/p1656356017949539?thread_t... this made it not a blocker for 10.0.x, since there's a chance it'll still be supported when 10.0.x goes EOL. However, there is still no publicly announced EOL date or intention to give X years of notice for jQuery UI, and we still want to remove jQuery entirely as a dependency too. |
| Gábor Hojtsy (he/him) | Could maybe @Kristen Pol (she/her) or @hestenet (he/him) take holding the meeting for next week? :pray: I’ll be back for the week after promise :slightly_smiling_face: |
| hestenet (he/him) | Mon July 4th is the Independence Day holiday here in the US so I think many folks will be out. |
| hestenet (he/him) | However I could try to set myself an alarm to hop in if needed. |
| Gábor Hojtsy (he/him) | Hah, and aw that would be great if you could. There are lots of attendees from around the globe here I think. :) (edited) |
| Kristen Pol (she/her) | I can run the meeting since we won't be going anywhere during the time |
| Kristen Pol (she/her) | I'll add a couple alarms :slightly_smiling_face: |
| Kristen Pol (she/her) | @hestenet (he/him) I got this one ^ Sleep in :slightly_smiling_face: |
| hestenet (he/him) | :thankful: |
| Gábor Hojtsy (he/him) | Thanks both! |
Comments
Comment #2
gábor hojtsyComment #15
gábor hojtsySaving notes.
Comment #16
gábor hojtsyThanks all!