Account created on 4 January 2013, about 12 years ago
  • Staff Software Engineer at Acquia 
#

Merge Requests

More

Recent comments

🇮🇳India vishalkhode

Thanks @rajeshreeputra for working on this. Changes looks good. Hence, merged.

🇮🇳India vishalkhode

Thanks @vipin.mittal18 & @Rajeshreeputra for reporting issues. The CI was failing earlier, I've updated the CI template and rebase the MR, though there are couple of allowed failure CI jobs and some of them are failing, we can create a separate ticket to fix this.

🇮🇳India vishalkhode

Reviewed changes, looks good to me. Hence, RTBC.

🇮🇳India vishalkhode

vishalkhode made their first commit to this issue’s fork.

🇮🇳India vishalkhode

Reviewed changes, looks good to me. We can remove temp commit and request for merge. Hence, marking RTBC.

🇮🇳India vishalkhode

Reviewed changes, looks good to me. Hence, RTBC.

🇮🇳India vishalkhode

Reviewed changes, looks good to me. Hence, RTBC.

🇮🇳India vishalkhode

Reviewed changes, looks good to me now. Hence, RTBC.

🇮🇳India vishalkhode

Reviewed MR !49, looks good to me now. Hence, RTBC.

🇮🇳India vishalkhode

Reviewed changes, looks good to me assuming 400 status code returned by Widen DAM only in case of scenarios where assets are being deleted/removed directly on Widen DAM portal.
Hence, RTBC.

🇮🇳India vishalkhode

Thanks everyone for working on this. Fixed in 9ddfc and released as part of 1.4.5 .

🇮🇳India vishalkhode

Hi @gausarts
Thanks for your suggestions, duly noted. I realize that I mistakenly updated the service from library.discovery to library.discovery.collector, which is now deprecated. I believe this would only break in Drupal 12 (which hasn’t been released yet), correct? That’s why we have the review system on drupal.org—to ensure that reviewers can catch these mistakes, suggest changes, and help get them fixed before merging. I agree that mistakes happen, but that’s where the review process comes in to catch and correct them.

The bigger issue here, in my opinion, is not that developers make incorrect changes in their MRs—nobody sets out to do that. The main problem is the absence of CI for this module. If we had CI, it would automatically test the module across all supported Drupal Core versions, flagging any test failures, deprecations, or code standards issues. Without this in place, it’s easy to merge changes that haven’t been fully tested across the different core versions.
Thanks.

🇮🇳India vishalkhode

Thanks @Rajeshreeputra for working on this. The Config Filter module dependency has been removed as it's not required.

🇮🇳India vishalkhode

Mistakenly Drupal 11 changes have been pushed as part of this, hence reverting and will create a new minor release, supporting Drupal 11 and removal of Config Filter module dependency and it'll done as part of 📌 Automated Drupal 11 compatibility fixes for sitestudio_config_management Needs review . Thanks everyone.

🇮🇳India vishalkhode

Thanks @nord102. Fixed and release as part of 3.3.11 .

🇮🇳India vishalkhode

@ok4p1: Yes, we've release by this week.

🇮🇳India vishalkhode

Not able to reproduce the issue with latest release. Followed the steps as mentioned in #25 and also followed the steps that's added in the ticket. Feel free to re-open the ticket, if this issue still occurs and we've the steps to reproduce the issue.
Thanks.

🇮🇳India vishalkhode

Reviewed changes, looks good to me now. Hence, RTBC.

🇮🇳India vishalkhode

There are couple of failures in Next Major CI, which are un-related to this issue, hence, I've not fixed this as part of this ticket. But If needed/requested, I'll fix those here. Hence, moving it to Needs Review.

🇮🇳India vishalkhode

vishalkhode changed the visibility of the branch 3471672-the-module-fails to active.

🇮🇳India vishalkhode

vishalkhode changed the visibility of the branch 3471672-the-module-fails to hidden.

🇮🇳India vishalkhode

@ankitv18 It's working fine now when specifying using the full namespace. The problem/issue seems to me is due to the PHP mocking, because we've created a mock of HttpKernelInterface interface in previous line and I believe the defined() check might referring to the mocked version of the class and it doesn't have the class constant defined. Hence, defined method was returning false.
Rest all changes looks good to me. Hence, RTBC.

🇮🇳India vishalkhode

Overall changes looks good to me. One minor change requested, please take a look. Hence, moving it back.

🇮🇳India vishalkhode

Reviewed changes, looks good to me. Hence, RTBC.

🇮🇳India vishalkhode

Reviewed changes, looks good to me. Hence, RTBC.

🇮🇳India vishalkhode

Making dataProviders to static in PHPUnit tests is must requirement for D11 compatibility. Hence, this needs to fixed first.

🇮🇳India vishalkhode

The Scheduler module has now 2.x-dev dev release available, so we can start working on this. Hence, re-opening this.

🇮🇳India vishalkhode

Ok, so the error was there, because I fixed the error, but didn't notice that the same error was added to ignore in PHPStan baseline file. But now I've updated the baseline file and removed that portion and so everything looks good now. Hence, requesting review.

But now, I can see different PHPStan errors in Current and Next Major Drupal Core version. Ex: https://git.drupalcode.org/issue/scheduler-3467349/-/pipelines/253073.

🇮🇳India vishalkhode

@jonathan1055: Ok, so I tweaked the method formatOutputText() and converted the regular function to an anonymous function (or closure) and assigned to the $formatOutputText variable. This allows us to use it like a regular function within the resetFormDisplayFields method. Because anyways defining a function inside another function or method, is I think generally not a good practice in PHP.

But now, I can see different PHPStan errors in Current and Next Major Drupal Core version. Ex: https://git.drupalcode.org/issue/scheduler-3467349/-/pipelines/253073
But I strongly think, this shouldn't have been caused due to my changes: https://git.drupalcode.org/issue/scheduler-3467349/-/compare/2.x...34673...

🇮🇳India vishalkhode

Further debugging, I found that the code in mglaman/phpstan-drupal extension is causing the issue. When I downgraded the mglaman/phpstan-drupal extension from 1.2.12 to 1.2.11, the error doesn't show up. I think @mglaman can help us here.

🇮🇳India vishalkhode

Hi @jonathan1055
Yes, I've not pushed anything, just got the fork push access and re-ran the job to see if maybe it's due to some issue of CI, but unfortunately it's not, but somehow I'm able to reproduce the issue, following is the stack trace I'm seeing:

-- ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ 
     Error                                                                                                                                                                                                                               
 -- ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ 
     Internal error: Method Drupal\scheduler\SchedulerManager::Drupal\scheduler\formatOutputText() does not exist while analysing file /Users/vishal.khode/drupal11/scheduler/src/SchedulerManager.php                                   
     Post the following stack trace to https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml:                                                                                                                           
     ## phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflection/Adapter/ReflectionClass.php(161)                                                          
     #0 /Users/vishal.khode/drupal11/scheduler/vendor/mglaman/phpstan-drupal/src/DeprecatedScope/IgnoreDeprecationsScope.php(31): PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass->getMethod()                               
     #1 /Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan-deprecation-rules/src/Rules/Deprecations/DeprecatedScopeHelper.php(24): mglaman\PHPStanDrupal\DeprecatedScope\IgnoreDeprecationsScope->isScopeDeprecated()         
     #2 /Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan-deprecation-rules/src/Rules/Deprecations/TypeHintDeprecatedInFunctionSignatureRule.php(40): PHPStan\Rules\Deprecations\DeprecatedScopeHelper->isScopeDeprecated()  
     #3 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(120): PHPStan\Rules\Deprecations\TypeHintDeprecatedInFunctionSignatureRule->processNode()                        
     #4 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Node/ClassStatementsGatherer.php(109): PHPStan\Analyser\FileAnalyser->PHPStan\Analyser\{closure}()                                         
     #5 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(647): PHPStan\Node\ClassStatementsGatherer->__invoke()                                                      
     #6 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(571): PHPStan\Analyser\NodeScopeResolver::PHPStan\Analyser\{closure}()                                      
     #7 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(458): PHPStan\Analyser\NodeScopeResolver->processStmtNode()                                                 
     #8 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(646): PHPStan\Analyser\NodeScopeResolver->processStmtNodes()                                                
     #9 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(458): PHPStan\Analyser\NodeScopeResolver->processStmtNode()                                                 
     #10 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(783): PHPStan\Analyser\NodeScopeResolver->processStmtNodes()                                               
     #11 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(458): PHPStan\Analyser\NodeScopeResolver->processStmtNode()                                                
     #12 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(753): PHPStan\Analyser\NodeScopeResolver->processStmtNodes()                                               
     #13 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(423): PHPStan\Analyser\NodeScopeResolver->processStmtNode()                                                
     #14 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(179): PHPStan\Analyser\NodeScopeResolver->processNodes()                                                        
     #15 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Command/WorkerCommand.php(139): PHPStan\Analyser\FileAnalyser->analyseFile()                                                              
     #16 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/evenement/evenement/src/EventEmitterTrait.php(111): PHPStan\Command\WorkerCommand::PHPStan\Command\{closure}()                         
     #17 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/clue/ndjson-react/src/Decoder.php(117): _PHPStan_b7fe9900d\Evenement\EventEmitter->emit()                                              
     #18 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/evenement/evenement/src/EventEmitterTrait.php(111): _PHPStan_b7fe9900d\Clue\React\NDJson\Decoder->handleData()                         
     #19 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/react/stream/src/Util.php(62): _PHPStan_b7fe9900d\Evenement\EventEmitter->emit()                                                       
     #20 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/evenement/evenement/src/EventEmitterTrait.php(111): _PHPStan_b7fe9900d\React\Stream\Util::_PHPStan_b7fe9900d\React\Stream\{closure}()  
     #21 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/react/stream/src/DuplexResourceStream.php(168): _PHPStan_b7fe9900d\Evenement\EventEmitter->emit()                                      
     #22 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/react/event-loop/src/StreamSelectLoop.php(201): _PHPStan_b7fe9900d\React\Stream\DuplexResourceStream->handleData()                     
     #23 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/react/event-loop/src/StreamSelectLoop.php(173): _PHPStan_b7fe9900d\React\EventLoop\StreamSelectLoop->waitForStreamActivity()           
     #24 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/src/Command/WorkerCommand.php(99): _PHPStan_b7fe9900d\React\EventLoop\StreamSelectLoop->run()                                                 
     #25 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Command/Command.php(259): PHPStan\Command\WorkerCommand->execute()                                                     
     #26 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(870): _PHPStan_b7fe9900d\Symfony\Component\Console\Command\Command->run()                              
     #27 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(261): _PHPStan_b7fe9900d\Symfony\Component\Console\Application->doRunCommand()                         
     #28 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/vendor/symfony/console/Application.php(157): _PHPStan_b7fe9900d\Symfony\Component\Console\Application->doRun()                                
     #29 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan(124): _PHPStan_b7fe9900d\Symfony\Component\Console\Application->run()                                                             
     #30 phar:///Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan.phar/bin/phpstan(125): _PHPStan_b7fe9900d\{closure}()                                                                                              
     #31 /Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan/phpstan(8): require('...')                                                                                                                                        
     #32 /Users/vishal.khode/drupal11/scheduler/vendor/bin/phpstan(119): include('...')                                                                                                                                                  
     #33 {main}                                                                                                                                                                                                                          
 -- ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ 


                                                                                                                        
 [ERROR] Found 1 error                                                                                                  
                                                                                                                        

PHP runtime version: 8.3.6
PHP version for analysis: 8.3.6 (from runtime)

PHPStan version: 1.11.10
PHPStan running from:
/Users/vishal.khode/drupal11/scheduler/vendor/phpstan/phpstan

Extension installer:
composer/composer: 2.7.7
composer/pcre: 3.2.0
mglaman/phpstan-drupal: 1.2.12
phpstan/phpstan-deprecation-rules: 1.2.0
phpstan/phpstan-phpunit: 1.4.0

Discovered Composer project root:
/Users/vishal.khode/drupal11/scheduler

Parallel processing scheduler:
# of detected CPU cores:   12
# of analysed files:       1
# of jobs:                 1
# of spawned processes:    1

⚠️  Result is incomplete because of severe errors. ⚠️
   Fix these errors first and then re-run PHPStan
   to get all reported errors.

Elapsed time: 7 seconds
Used memory: 128.5 MB

🇮🇳India vishalkhode

Please create MR for the same. I'll review and merge as it's very small fix. Hence, moving it back.

🇮🇳India vishalkhode

Thanks everyone for working on this. Looks good to me, Hence, merged.

🇮🇳India vishalkhode

I ran the ESlint in local and it showed errors in config_update_ui.settings.yml file. I think we should handle & fix that in a separate ticket. So, I'm Ok, if we've skipped the ESlint job here, we can enable and fix those separately. Thanks.

🇮🇳India vishalkhode

Requested some changes, hence moving it back to Needs Work. Also, let's not skip eslint and fix that as well. If that's a major change and required more changes, we can skip eslint and cslint for now and handle that in a separate ticket and limit this ticket for PHPCS and PHPStan fixes only.

Production build 0.71.5 2024