Problem/Motivation

drupal-check results on commit hash:
source : [git] https://git.drupal.org/project/environment_indicator 49ebddedfd4402ed77241dada5a733686d9df336
source : http://cgit.drupalcode.org/environment_indicator


 ------ --------------------------------------------------- 
  Line   src/EnvironmentIndicatorForm.php                   
 ------ --------------------------------------------------- 
  63     Call to deprecated function drupal_set_message().  
 ------ --------------------------------------------------- 

 ------ --------------------------------------------------- 
  Line   src/Form/EnvironmentIndicatorDeleteForm.php        
 ------ --------------------------------------------------- 
  58     Call to deprecated function drupal_set_message().  
 ------ --------------------------------------------------- 

 [ERROR] Found 2 errors                                                     

 

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

mcdwayne created an issue. See original summary.

omrmankar’s picture

Please review the code.

jerdavis’s picture

StatusFileSize
new3.16 KB

Updating patch to include drupalci.yml and a dependency on Drupal Core 8.5 or greater.

jerdavis’s picture

Status: Active » Needs review
WidgetsBurritos’s picture

Status: Needs review » Reviewed & tested by the community

+1 RTBC

Although given that it doesn't appear DrupalCI is turned on for this module, patch #2 seems sufficient here, unless the maintainers want to turn that on.

omrmankar’s picture

@WidgetsBurritos Thanks For Appreciation :)

lisa.rae’s picture

Status: Reviewed & tested by the community » Needs work

Patch #2 generates a syntax error.

The ternary for line 58 is checking for lack of a value in variable $change, but $change is never initialized so it will always evaluate to "true". However syntactically the ternary is missing the "else" component which is causing the syntax error.

omrmankar’s picture

Status: Needs work » Needs review
omrmankar’s picture

omrmankar’s picture

StatusFileSize
new1.41 KB
omrmankar’s picture

bblake’s picture

Patch in #10 has a syntax error. Corrected patch attached.

lisa.rae’s picture

Issue tags: +Drupal 9 deprecated code report
lisa.rae’s picture

StatusFileSize
new2.24 KB

A better implementation, rather than calling the static Drupal core messenger service, is to implement this as a trait.

Patch is attached.

jayasree.e’s picture

StatusFileSize
new1.41 KB

Both the classes have messenger trait by default. So used $this->messenger().Please review.

malaynayak’s picture

Status: Needs review » Reviewed & tested by the community

Patch #15 looks good.

avpaderno’s picture

Issue tags: -Drupal 9 deprecated code report
matroskeen’s picture

StatusFileSize
new2.33 KB

The last patch looks good, however, we also need to add core_version_requirement property to be Drupal 9 compatible.

heddn’s picture

+1 on RTBC. I did a quick run through the code of the module. It seems to use basic form api and theming functionality. This hasn't changed much between D8 and D9. I didn't find any deprecations. So listing core_version_requirement: ^8 || ^9 is quite appropriate.

joelpittet’s picture

RTBC++

johan den hollander’s picture

Tested the #18 on 3.x-dev. The upgrade status now shows Environment indicator as fully compatible with D9.
RTBC!

pratik_kamble’s picture

@Matroskeen, Thanks for the patch. I have run the Deprecation report against the module no error/warning found. Patch LGTM.

pratik_kamble’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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