Drupal 9 is coming! We need to make sure that Configuration Update Manager module is fully Drupal 9 compatible.

Tasks:
- Run phpstan or other analysis tools to see if we are calling any deprecated code or have other D9 compatibility problems.
- If so, fix them in a patch.

See also:
#3042621: Refactor to not support Drush 8 any more

CommentFileSizeAuthor
#9 3089420-8.patch2.09 KBSudeepthi Peteti
#8 interdiff_6-8.txt3.17 KBSudeepthi Peteti
#6 3089420-6.patch887 byteschr.fritsch

Comments

jhodgdon created an issue. See original summary.

samaphp’s picture

samaphp
Primary language Arabic
Location Riyadh, SA 🇸🇦
commented

Thank you all for the efforts, I'm working on this in DrupalCon Amsterdam 2019

I tried to run drupal-check against latest 1.x-dev and I found these errors. I'm working on it now.

% drupal-check config_update
 27/27 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------ 
  Line   config_update_ui/config_update_ui.drush_testing.inc                                                              
 ------ ------------------------------------------------------------ 
         Class DrushLogLevel was not found while trying to analyse it - autoloading is probably not configured properly.  
 ------ ------------------------------------------------------------ 

 ------ ---------------------------------------------------------- 
  Line   config_update_ui/src/Commands/ConfigUpdateUiCommands.php  
 ------ ---------------------------------------------------------- 
  31     Call to deprecated method logger() of class Drush\Drush:  
         Use injected logger instead.                              
 ------ ---------------------------------------------------------- 

 ------ ------------------------------------------------------------ 
  Line   config_update_ui/src/Logger/ConfigUpdateUiDrush8Logger.php  
 ------ ------------------------------------------------------------ 
  81     Call to deprecated function drush_log().                    
  93     Call to deprecated function drush_print().                  
 ------ ------------------------------------------------------------

Drupal 8.7.8
Drupal Check 1.0.12

samaphp’s picture

samaphp
Primary language Arabic
Location Riyadh, SA 🇸🇦
commented

As per the explaination provided here in this comment on issue #3042621 We can ignore the first error.

I tried to inject the logger but it takes more time :( I just noticed the logger is a DrushLog, not the Drupal logger. So, be careful to inject the right logger.
Also, be aware that (tests) does not test the class of Drush commands ConfigUpdateUiCommands. So, please make sure to run this command to test your changes manually drush config-list-types and to make sure your changes did not break the Drush commands for this useful module.

Thanks!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Postponed

This should be postponed until the related issue is fixed (removing Drush 8 support).

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

So can we un-postpone here?

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new 3089420-6.patch887 bytes

It's save to commit this and would help other modules that depend on config_update to get ready for D9 as well.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Postponed » Needs review

You're right, we should probably un-postpone and commit that at least.

Sudeepthi Peteti’s picture

Sudeepthi Peteti commented
StatusFileSize
new interdiff_6-8.txt3.17 KB
Sudeepthi Peteti’s picture

Sudeepthi Peteti commented
StatusFileSize
new 3089420-8.patch2.09 KB
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

We're not ready to commit the patch in #9, which is about Drush, and is being handled on #3042621: Refactor to not support Drush 8 any more.

I'll launch a test vs. Drupal 9 of the patch in #6.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

DrupalCI will not work with the changed core requirement in a patch. It needs to be in the main branch before DrupalCI can pick it up.

So we need to run the tests locally and then commit it.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

OK, if someone would like to do that, I would appreciate the help to move this forward. Thanks!

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

I ran the tests locally for D8.9 and 9 without any failures.

  • jhodgdon committed b286ac9 on 8.x-1.x 
    Issue #3089420 by chr.fritsch, samaphp: Make sure Config Update Manager...
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Excellent! Thanks very much for doing that. I've committed this patch.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

Thank you. When are you planning to create a new release?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Good idea -- new release just created -- it should be published on drupal.org within an hour.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.