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
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | 3042790-18.patch | 2.33 KB | matroskeen |
| #15 | 3042790-15.patch | 1.41 KB | jayasree.e |
| #14 | 3042790-14.patch | 2.24 KB | lisa.rae |
| #12 | drupal_9_deprecated_code_report-3042790-12.patch | 1.41 KB | bblake |
| #10 | drupal_9_deprecated_code_report-3042790-10.patch | 1.41 KB | omrmankar |
Comments
Comment #2
omrmankarPlease review the code.
Comment #3
jerdavisUpdating patch to include drupalci.yml and a dependency on Drupal Core 8.5 or greater.
Comment #4
jerdavisComment #5
WidgetsBurritos commented+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.
Comment #6
omrmankar@WidgetsBurritos Thanks For Appreciation :)
Comment #7
lisa.rae commentedPatch #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.
Comment #8
omrmankarComment #9
omrmankarComment #10
omrmankarComment #11
omrmankarComment #12
bblake commentedPatch in #10 has a syntax error. Corrected patch attached.
Comment #13
lisa.rae commentedComment #14
lisa.rae commentedA better implementation, rather than calling the static Drupal core messenger service, is to implement this as a trait.
Patch is attached.
Comment #15
jayasree.e commentedBoth the classes have messenger trait by default. So used $this->messenger().Please review.
Comment #16
malaynayak commentedPatch #15 looks good.
Comment #17
avpadernoComment #18
matroskeenThe last patch looks good, however, we also need to add core_version_requirement property to be Drupal 9 compatible.
Comment #19
heddn+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 || ^9is quite appropriate.Comment #20
joelpittetRTBC++
Comment #21
johan den hollander commentedTested the #18 on 3.x-dev. The upgrade status now shows Environment indicator as fully compatible with D9.
RTBC!
Comment #22
pratik_kamble@Matroskeen, Thanks for the patch. I have run the Deprecation report against the module no error/warning found. Patch LGTM.
Comment #24
pratik_kamble