Drupal 9 Readiness

This is a good candidate for Drupal Global Contribution Weekend 2020

1. +++ b/src/Form/DatalayerSettingsForm.php
   @@ -174,10 +174,10 @@ class DatalayerSettingsForm extends ConfigFormBase {
   +    if (empty($this->requestStack->getCurrentRequest()->request) && $helper && !file_exists(DRUPAL_ROOT . $path)) {
   

   This won't work because $this->requestStack has not been defined in this object. It would need to be injected via the __construct() and create() methods.

2. +++ b/src/Form/DatalayerSettingsForm.php
   @@ -174,10 +174,10 @@ class DatalayerSettingsForm extends ConfigFormBase {
   +          '%filepath' => $path,
   +          ':helper' => 'https://github.com/google/data-layer-helper',
   

   These whitespace changes aren't necessary and actually introduce new coding style failures. See https://www.drupal.org/pift-ci-job/1554143

I've included a patch that adds a test to @uberhacker's patch.

@rksyravi,

• "I understand that $this->requestStack is not defined in the current class but it is available in the parent class. class DatalayerSettingsForm extends ConfigFormBase >> abstract class ConfigFormBase extends FormBase >> in this class protected $requestStack; is used. Check this path for your confirmation /web/core/lib/Drupal/Core/Form/FormBase.php file."

  I'm attaching a test that proves that this is not actually getting instantiated as it generates this error:

  1) Drupal\Tests\datalayer\Functional\DataLayerFunctionalTest::testAdminSettingsForm
  Exception: Error: Call to a member function getCurrentRequest() on null
  Drupal\datalayer\Form\DatalayerSettingsForm->buildForm()() (Line: 177)

• "These are just part of the code formatting and Drupal coding standards."
  If you view the link I posted, you'll see that this is actually introducing 2 new coding standard problems, so no we don't want this.
  
• "Simply removing the variable is not an option, if the dev has used and check for empty then there must be some need according to his requirement."

  As a maintainer of this module, I've worked with @uberhacker separately, along with a few other people at our meetup to determine that is not actually needed based on the context of what is there. So we've decided just removing it is the best course of action.

Well no I didn't attach the new patch on that previous comment. Let's try that again.

Adding credit for cutehair from the Austin Contribution Weekend meetup discussion

Adding credit for rocketeerbkw from the Austin Contribution Weekend meetup discussion

Somehow those patches got corrupted. Trying this again.

Based on the (very limited) test coverage, D9 should work based on the current deprecations. Obviously D9 is still a moving target, and may change but we appear to have coverage for now: