Added Rector and Drupal Check and fixed Drupal 10 compatibility. #250

AlexSkrypnyk · 2022-10-06T15:33:52Z

Added Rector
Added Drupal Check
Fixed code that was not compatible with Drupal 10
Fixed compatibility with Symfony\Process in Drush Driver (copied from Fixed compatibility with Symfony\Process v4+ for Drush driver. #251 - sorry, had to keep it in this branch for downstream projects)

Please note that Rector and Drupal check are only scanning Drupal 8 driver.

…h driver.

pfrenssen · 2022-10-13T11:09:35Z

.travis.yml

@@ -1,8 +1,6 @@
 language: php

 php:
-  - 7.4
-  - 8.0


Any reason to stop testing on PHP 8.0? It is still supported until October 2023.

I think for as long as we declare in composer.json to be compatible with PHP 7.4 we should also keep testing on 7.4. Probably we shouldn't drop PHP 7.4 support unless we plan to release a new major version.

I suppose the ./vendor/bin/drupal-check test only works on PHP 8.1, but we can still run the remainder of the suite on older PHP versions.

Drupal 10's requirement is to use 8.1. The CI may fail for 8.0 on D10.

Let me try to get this back and see how CI reacts.

PHP8 did not work

Your requirements could not be resolved to an installable set of packages. Problem 1 - drupal/core-recommended[10.0.0-beta1, ..., 10.0.0-beta2] require symfony/process ~v6.1.3 -> satisfiable by symfony/process[v6.1.3]. - symfony/process[v6.1.0-BETA1, ..., v6.1.3] require php >=8.1 -> your php version (8.0.23) does not satisfy that requirement. - Root composer.json requires drupal/core-recommended ^10@beta -> satisfiable by drupal/core-recommended[10.0.0-beta1, 10.0.0-beta2]. The command "composer install" failed and exited with 2 during .

https://app.travis-ci.com/github/jhedstrom/DrupalDriver/jobs/585619255

I've reverted my change so that we are only testing on PHP 8.1

If there is a different way to test with PHP 8.0 - please let me know and I will update this PR.

pfrenssen · 2022-10-13T11:17:47Z

composer.json

-      "phpcs --standard=./phpcs-ruleset.xml ."
-
+      "phpcs --standard=./phpcs-ruleset.xml --ignore=drupal .",


Let's not add the --ignore option inline. Let's instead update the ignored paths in phpcs-ruleset.xml:

<exclude-pattern>./drupal/*</exclude-pattern>

pfrenssen · 2022-10-13T11:33:46Z

src/Drupal/Driver/Cores/Drupal8.php

-  public function expandEntityBaseFields($entity_type, \stdClass $entity, array $base_fields) {
+  public function expandEntityBaseFields($entity_type, $entity, array $base_fields) {


Why remove the \stdClass type hint? This is here to clarify that this is not a real entity but rather a "mocked" value object. It is literally a \stdClass object so the type hint is correct. Am I missing something?

This, as well as other changes, is a result of Rector's and drupal-check report. I don't remember what static analysis rule this was violating specifically, but it would not pass those tools.

We can revert and add an ignore comment for those tools to ignore this.

Please advise

src/Drupal/Driver/Fields/Drupal8/ImageHandler.php

src/Drupal/Driver/Cores/Drupal8.php

AlexSkrypnyk · 2022-10-13T22:28:05Z

@pfrenssen
Thank you for reviewing this. Agree with all comments. I will address them shortly. Thanks

…Drupal versions.

This reverts commit 2143a50.

AlexSkrypnyk · 2022-10-13T23:38:54Z

@pfrenssen
I have addressed all the comments. Could you please review again. Thank you

jhedstrom · 2022-10-17T18:40:39Z

re: testing on previous PHP versions

I think it is these lines that are causing those to not work:

    "drupal/core-composer-scaffold": "^10@beta",
    "drupal/core-recommended": "^10@beta",

They can probably either be removed, or switched to allow the older versions of Drupal as in the require section:

    "drupal/core-utility": "^8.4 || ^9 || ^10.@beta"

AlexSkrypnyk · 2022-10-18T06:21:33Z

@jhedstrom
could you please advise what do they have to be switched to?

jhedstrom · 2022-10-24T22:53:39Z

I think they just need to allow 8.4 and 9 like we do for the core-utility:

    "drupal/core-composer-scaffold": "^8.4 || ^9 || ^10@beta",
    "drupal/core-recommended": "^8.4 || ^9 || ^10@beta",

(we could probably drop 8.x for now too, and then drop 9 when it is eol.)

This reverts commit 6da53e8.

AlexSkrypnyk · 2022-10-24T23:44:29Z

thank you @jhedstrom
I've updated the configs, but ran out of OSS credits on Travis CI (can we switch to Circle or GH actions?). Not sure how to test this without those credits. Probably have to wait till Nov 1.

jhedstrom · 2022-10-24T23:50:29Z

@AlexSkrypnyk I've opened #254 to track that. It should be a relatively small effort.

Also, there's #249 that might be relevant to the version changes happening here for core libraries.

# Conflicts: # .travis.yml

AlexSkrypnyk · 2022-11-23T06:37:01Z

@claudiu-cristea @jhedstrom
I've updated this PR with the latest changes from master and updated CI to include D10. Could you please review. Thank you

jhedstrom · 2022-11-23T18:47:40Z

Thanks!

Kingdutch · 2023-06-22T08:08:19Z

@jhedstrom This PR introduced a conflict with older Drupal versions which seems like something that shouldn't happen in a patch release (but should be a major version release instead). This prevents upgrading in some instances.

See goalgorilla/open_social_dev#51 for the conflict message that this causes.

Alex Skrypnyk added 5 commits October 7, 2022 02:31

Added Rector and Drupal check.

5a98674

Fixed Drupal 10 code compatibility.

8177f64

Updated CI matrix to use only PHP 8.1 supported by Drupal 10.

3e746a2

Updated mockery version.

aee6f67

Fixed Recotr config.

e45edff

AlexSkrypnyk force-pushed the feature/drupal-10 branch from fbaa754 to e45edff Compare October 6, 2022 15:47

[jhedstrom#251] Fixed compatibility with Symfony\Process v4+ for Drus…

0fda7a9

…h driver.

pfrenssen requested changes Oct 13, 2022

View reviewed changes

Alex Skrypnyk added 5 commits October 14, 2022 09:37

Moved PHPCS exclusion to the phpcs config file.

16963d8

Added conflict to composer.json to indicate incompatibility with old …

4bfaa47

…Drupal versions.

Reverted argument type changes and updated docblocks.

4499c71

Added back testing using PHP 8.0.

2143a50

Revert "Added back testing using PHP 8.0."

6da53e8

This reverts commit 2143a50.

AlexSkrypnyk requested a review from pfrenssen October 13, 2022 23:39

AlexSkrypnyk added 2 commits October 25, 2022 10:25

Revert "Revert "Added back testing using PHP 8.0.""

eb018dd

This reverts commit 6da53e8.

Updated core and recommended packages to allow lower versions.

282ff13

AlexSkrypnyk added 2 commits November 23, 2022 17:34

Merge branch 'master' into feature/drupal-10

5809a72

# Conflicts: # .travis.yml

Updated GH actions to include D10 version.

e033ac9

AlexSkrypnyk requested review from claudiu-cristea and removed request for pfrenssen November 23, 2022 06:36

AlexSkrypnyk added 2 commits November 23, 2022 17:46

Added phpunit-bridge.

b17388f

Exclude D10 from building on 8.0.

03ae7c9

AlexSkrypnyk requested review from pfrenssen and removed request for claudiu-cristea November 23, 2022 06:49

jhedstrom merged commit 6057ebd into jhedstrom:master Nov 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Rector and Drupal Check and fixed Drupal 10 compatibility. #250

Added Rector and Drupal Check and fixed Drupal 10 compatibility. #250

AlexSkrypnyk commented Oct 6, 2022 •

edited

Loading

pfrenssen Oct 13, 2022

AlexSkrypnyk Oct 13, 2022 •

edited

Loading

AlexSkrypnyk Oct 13, 2022

pfrenssen Oct 13, 2022

pfrenssen Oct 13, 2022

AlexSkrypnyk Oct 13, 2022

AlexSkrypnyk commented Oct 13, 2022

AlexSkrypnyk commented Oct 13, 2022

jhedstrom commented Oct 17, 2022

AlexSkrypnyk commented Oct 18, 2022

jhedstrom commented Oct 24, 2022

AlexSkrypnyk commented Oct 24, 2022

jhedstrom commented Oct 24, 2022

AlexSkrypnyk commented Nov 23, 2022

jhedstrom commented Nov 23, 2022

Kingdutch commented Jun 22, 2023

		"phpcs --standard=./phpcs-ruleset.xml ."

		"phpcs --standard=./phpcs-ruleset.xml --ignore=drupal .",

		public function expandEntityBaseFields($entity_type, \stdClass $entity, array $base_fields) {
		public function expandEntityBaseFields($entity_type, $entity, array $base_fields) {

Added Rector and Drupal Check and fixed Drupal 10 compatibility. #250

Added Rector and Drupal Check and fixed Drupal 10 compatibility. #250

Conversation

AlexSkrypnyk commented Oct 6, 2022 • edited Loading

pfrenssen Oct 13, 2022

Choose a reason for hiding this comment

AlexSkrypnyk Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

AlexSkrypnyk Oct 13, 2022

Choose a reason for hiding this comment

pfrenssen Oct 13, 2022

Choose a reason for hiding this comment

pfrenssen Oct 13, 2022

Choose a reason for hiding this comment

AlexSkrypnyk Oct 13, 2022

Choose a reason for hiding this comment

AlexSkrypnyk commented Oct 13, 2022

AlexSkrypnyk commented Oct 13, 2022

jhedstrom commented Oct 17, 2022

AlexSkrypnyk commented Oct 18, 2022

jhedstrom commented Oct 24, 2022

AlexSkrypnyk commented Oct 24, 2022

jhedstrom commented Oct 24, 2022

AlexSkrypnyk commented Nov 23, 2022

jhedstrom commented Nov 23, 2022

Kingdutch commented Jun 22, 2023

AlexSkrypnyk commented Oct 6, 2022 •

edited

Loading

AlexSkrypnyk Oct 13, 2022 •

edited

Loading