drupal-graphql / graphql

GraphQL integration for Drupal 9/10
288 stars 202 forks source link

Drupal 11 readiness. #1407

Closed apathak18 closed 2 weeks ago

apathak18 commented 2 months ago

Prepare drupal-graphql module ready for Drupal 11

klausi commented 2 months ago

Thanks, if we start to support Drupal 11 we should enable it in our testing. Can you enable it in testing.yml?

apathak18 commented 2 months ago

Thanks, if we start to support Drupal 11 we should enable it in our testing. Can you enable it in testing.yml?

@klausi Initial change in the workflow to include D10.3.x and D11.0.x Now Jobs are failing and will push changes as per the failed jobs.

klausi commented 1 month ago

I just merged #1410 to enable Drupal 10.3 testing. So in this PR we only need to add one extra run for Drupal 11 without PHPStan.

Kingdutch commented 1 month ago

Thanks for opening this!

I do believe we should split this PR up into multiple PRs and possibly over multiple releases.

For example the webonyx/graphql-php dependency was flagged already as breaking change in https://github.com/drupal-graphql/graphql/issues/1335 so if that's a blocker for Drupal 11 then we likely need to release GraphQL 5.0.0 with Drupal 10.x and 11 compatibility.

We should also evaluate the other dependency upgrades and determine whether they're needed or nice to have. Ideally doing them in a way where extending modules can focus on one change at a time.

I think it's important for a smooth upgrade experience that we apply Symfony's upgrade philosophy here. We should ensure that consumers can always focus on upgrading one thing at a time. For example, they can focus on upgrading the PHP version and then separately the GraphQL package before making the jump to Drupal 11. Or Drupal 11 first and then the GraphQL package.

I know from experience that modules that force adopters to upgrade multiple composer packages at the same time and/or the PHP version as well, are a lot harder than having only one of those be included in a release.

With the insights that you already have of what might need to be upgraded it'd be great to open up an issue that outlines which changes we need to make so we can plan the right approach.

apathak18 commented 1 month ago

@Kingdutch @klausi I've rebased the PR and revert the webnoyx package to v14 as v15 starts failing all tests.

Right now tests are passing for all D10.x.x versions except 11.0.x (Am I missing something or is there any configuration for database server which needs to be updated) See:https://github.com/drupal-graphql/graphql/actions/runs/9996316696/job/27630424860?pr=1407#step:11:1257

vishalkhode1 commented 1 month ago

@Kingdutch @klausi I've rebased the PR and revert the webnoyx package to v14 as v15 starts failing all tests.

Right now tests are passing for all D10.x.x versions except 11.0.x (Am I missing something or is there any configuration for database server which needs to be updated) See:https://github.com/drupal-graphql/graphql/actions/runs/9996316696/job/27630424860?pr=1407#step:11:1257

@apathak18 The tests are failing because of old version of Ubuntu, you should update .github/workflows/testing.yml file and change runs-on: ubuntu-latest to runs-on: ubuntu-24.04

apathak18 commented 1 month ago

@Kingdutch @klausi 11.0.x pipeline reports phpstan issues ~~ Mentioned below are Changelog of those deprecations:

  1. https://www.drupal.org/node/2999981 (format_size)
  2. https://www.drupal.org/node/3363700 (file_validate)

Can we bump the drupal minimum version to D10.2? Otherwise we need to handle these deprecations to support backward compatibility.

apathak18 commented 3 weeks ago

Some attachments based on validating the changes on local.

Screenshot 2024-07-25 at 10 54 01 PM Screenshot 2024-07-25 at 10 54 08 PM Screenshot 2024-07-25 at 10 54 21 PM
klausi commented 2 weeks ago

Merged #1415, please update your branch from 8.x-4.x, now you don't need any changes for AlterableSchemaTest.php.

apathak18 commented 2 weeks ago

@klausi All feedbacks are addressed ~~ also rebased & resolved the conflicts, now this one is ready for a review.

klausi commented 2 weeks ago

Please don't do force pushes, makes it harder for me to update local test branches. All the commits here don't matter, we will squash merge them aynway at the end.

apathak18 commented 2 weeks ago

Ohh I'll note this from ahead. For now I'm done with all the feedbacks, please check.

apathak18 commented 2 weeks ago

Excellent, all looking good now. Thank you for your patience! Thanks Quote of the day: Patience is a key to success!!