--------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
--------------------------------------------------------------------------
54 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
55 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
73 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
144 | WARNING | Unused variable $id.
-------------------------------------------------------------------------

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pavnish created an issue. See original summary.

pavnish’s picture

Issue summary: View changes

Interface Drupal\key\Plugin\KeyPluginInterface extends deprecated interface Drupal\Component\Plugin\ConfigurablePluginInterface:
Drupal\Component\Plugin\ConfigurablePluginInterface is deprecated
in Drupal 8.7.0 and will be removed before Drupal 9.0.0. You should implement
ConfigurableInterface and/or DependentPluginInterface directly as needed. If
you implement ConfigurableInterface you may choose to implement
ConfigurablePluginInterface in Drupal 8 as well for maximum compatibility,
however this must be removed prior to Drupal 9.

pavnish’s picture

Assigned: pavnish » Unassigned
Status: Active » Needs work
suzymasri’s picture

Updated drupal-check report:

65/65 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

------ ------------------------------------------------
Line /drush/key_delete.drush.inc
------ ------------------------------------------------
29 Call to deprecated function drush_log().
46 Call to deprecated function drush_log().
------ ------------------------------------------------

------ ------------------------------------------------------
Line drush/key_list.drush.inc
------ ------------------------------------------------------
19 Call to deprecated function _convert_csv_to_array():
Use \Drush\StringUtils::csvToArray
29 Call to deprecated function _convert_csv_to_array():
Use \Drush\StringUtils::csvToArray
------ ------------------------------------------------------

------ ----------------------------------------------
Line drush/key_save.drush.inc
------ ----------------------------------------------
36 Call to deprecated function drush_log().
97 Call to deprecated function drush_log().
------ ----------------------------------------------

------ ----------------------------------------------------------------------------
Line src/Plugin/KeyPluginInterface.php
------ ----------------------------------------------------------------------------
12 Interface Drupal\key\Plugin\KeyPluginInterface extends deprecated interface Drupal\Component\Plugin\ConfigurablePluginInterface:
Drupal\Component\Plugin\ConfigurablePluginInterface is deprecated
in Drupal 8.7.0 and will be removed before Drupal 9.0.0. You should implement
ConfigurableInterface and/or DependentPluginInterface directly as needed. If
you implement ConfigurableInterface you may choose to implement
ConfigurablePluginInterface in Drupal 8 as well for maximum compatibility,
however this must be removed prior to Drupal 9.
------ ----------------------------------------------------------------------------

------ ----------------------------------------------------------------------------
Line src/Plugin/KeyType/EncryptionKeyType.php
------ ----------------------------------------------------------------------------
110 Call to deprecated method randomBytes() of class Drupal\Component\Utility\Crypt:
in drupal:8.8.0 and is removed from drupal:9.0.0.
Use PHP's built-in random_bytes() function instead.
------ ----------------------------------------------------------------------------

[ERROR] Found 8 errors

suzymasri’s picture

suzymasri’s picture

Status: Needs work » Needs review
Arlina’s picture

#5 works against Drupal 9.0.0-beta2.

rlhawk’s picture

The work around ConfigurablePluginInterface was done in #3076926: Replace dependency on deprecated ConfigurablePluginInterface and is RTBC. I had committed it a while back, but it broke compatibility with Drupal versions <= 8.6, so it was reverted. Now that 8.6 is not longer supported, we can go forward with it. I will create a release that contains existing commits, which will be the last release that will work on 8.6. We should use core_version_requirement: ^8.7.0 || ^9 in the .info file.

rlhawk’s picture

Here's a rerolled patch with the duplicate changes from #3076926: Replace dependency on deprecated ConfigurablePluginInterface removed. It also adds core_version_requirement: ^8.7.0 || ^9 to the info.yml file and "drupal/core": "^8.7.0 || ^9" as a requirement in composer.json.

rlhawk’s picture

timmillwood’s picture

Running tests with PHP 7

Status: Needs review » Needs work

The last submitted patch, 10: 3116139-10.patch, failed testing. View results

Manuel Garcia’s picture

Version: 8.x-1.12 » 8.x-1.x-dev

Just wanted to point out that the deprecations on *.drush.inc are because these are the legacy Drush 8 commands and these have already been migrated to Drush 9 (see KeyCommands) so its up to the maintainers to decide when to drop support for Drush 8. Looking at this https://docs.drush.org/en/master/install/#drupal-compatibility one could make an argument that any time would in theory be ok.

heddn’s picture

+++ b/key.info.yml
@@ -1,6 +1,6 @@
+core_version_requirement: ^8.7.0 || ^9

Listing 8.7.0 won't work. The minimum is 8.7.7. And make sure when adding tests that we use the right version ranges of PHP. Drupal 8.8 requires 7.2. D9 requires 7.3.

Manuel Garcia’s picture

Status: Needs work » Needs review
FileSize
651 bytes
4.92 KB

Re #14 good points. Hopefully this will do the trick.

jcnventura’s picture

Status: Needs review » Needs work

Would it not be better to use $this->logger instead of \Drupal::logger in Drush commands?

jcnventura’s picture

Still missing this one:

12 Interface Drupal\key\Plugin\KeyPluginInterface extends deprecated interface Drupal\Component\Plugin\ConfigurablePluginInterface:
Drupal\Component\Plugin\ConfigurablePluginInterface is deprecated
in Drupal 8.7.0 and will be removed before Drupal 9.0.0. You should implement
ConfigurableInterface and/or DependentPluginInterface directly as needed. If
you implement ConfigurableInterface you may choose to implement
ConfigurablePluginInterface in Drupal 8 as well for maximum compatibility,
however this must be removed prior to Drupal 9.

Looking at the code, we need to extend ConfigurableInterface

pradeepjha’s picture

Assigned: Unassigned » pradeepjha
jcnventura’s picture

pradeepjha’s picture

Used $this->logger instead of \Drupal::logger on #15 patch based on #16 comment.

AaronBauman’s picture

#20 looks good to me

jcnventura’s picture

rishab.singh’s picture

Assigned: Unassigned » rishab.singh
rishab.singh’s picture

Assigned: rishab.singh » Unassigned
Status: Needs review » Reviewed & tested by the community

Hi @pradeepjha

I have tested this patch and moving this to RTBC. #20 Patch looks good to me!

Manuel Garcia’s picture

Thanks all, just queued tests to run with the versions we're adding support to to see where we stand.

jcnventura’s picture

Thanks for this. RTBC++

  • rlhawk committed 946e40b on 8.x-1.x authored by pradeepjha
    Issue #3116139 by Manuel Garcia, suzymasri, pradeepjha, jcnventura,...
rlhawk’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, everybody.

suzymasri’s picture

Status: Fixed » Needs review
FileSize
2.13 KB

@rlhawk Thanks for committing this!

I tested the dev branch and Travis is failing with below PHP compatibility issues related to using $this->logger that was added in #20:

FILE: .../key/drush/key_delete.drush.inc

--------------------------------------------------------------------------

FOUND 2 ERRORS AFFECTING 2 LINES

--------------------------------------------------------------------------

 27 | ERROR | "$this" can no longer be used in a plain function or method since PHP 7.1.

 44 | ERROR | "$this" can no longer be used in a plain function or method since PHP 7.1.

--------------------------------------------------------------------------


FILE: .../key/drush/key_save.drush.inc

--------------------------------------------------------------------------

FOUND 2 ERRORS AFFECTING 2 LINES

--------------------------------------------------------------------------

 35 | ERROR | "$this" can no longer be used in a plain function or method since PHP 7.1.

 96 | ERROR | "$this" can no longer be used in a plain function or method since PHP 7.1.

--------------------------------------------------------------------------

Patch attached to remove $this->logger and use \Drupal::logger instead.

pavnish’s picture

Assigned: Unassigned » pavnish
Status: Needs review » Needs work
Manuel Garcia’s picture

Status: Needs work » Reviewed & tested by the community

Thanks @suzymasri - the patch on 29 looks good to me.

pavnish’s picture

Assigned: pavnish » Unassigned

  • rlhawk committed 2f184d1 on 8.x-1.x authored by suzymasri
    Issue #3116139 by suzymasri, Manuel Garcia: Drupal 9 Readiness
    
rlhawk’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, @suzymasri.

Status: Fixed » Closed (fixed)

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

AaronBauman’s picture

Is there a release plan for stable D9 compatible version?
Any blockers I can help with?

I can't push a stable D9 release of my own module, which depends on Key, until Key has a stable D9 compatible release.