Adding D11 support for nextjs. #778
Conversation
|
@apathak18 is attempting to deploy a commit to the Chapter Three Team on Vercel. A member of the Team first needs to authorize it. |
|
@shadcn I'm not able to add the reviewer and other pipelines are blocked without reviewer. |
|
@shadcn could you please review this PR and check if its looks for D11 compatible release? |
|
@JohnAlbin, |
|
I've refractor the tests to move delete action log after revalidate as I've noticed it is logging after revalidating (Reason delete event fire on hook_entity_predelete) Tests output for D11 |
|
@JohnAlbin Please review this PR and let me know if any other changes required. |
| - drupal: "11.0.x" | ||
| php: "8.1" | ||
| - drupal: "11.0.x" | ||
| php: "8.2" |
There was a problem hiding this comment.
@yobottehg Correct!!
Pipeline is running on 8.3
see: 
this configuration is to exclude running with 8.1, 8.2 versions of php for Drupal 11
| '@action' => $action, | ||
| ]; | ||
|
|
||
| $this->assertNotEmpty(Database::getConnection()->select('watchdog', 'w') |
There was a problem hiding this comment.
@apathak18 I'm not sure on this, but we can use it like below instead of using Database class:
| $this->assertNotEmpty(Database::getConnection()->select('watchdog', 'w') | |
| $this->assertNotEmpty($this->container->get('database')->select('watchdog', 'w') |
There was a problem hiding this comment.
Generally things look good here. Thanks to all who contributed on this one.
In general, I'd encourage the scope of an upgrade issue like this to be kept just to things necessary for the upgrade - core version requirement in module info files, any deprecated code. There were some things along the way outside of that, and I still think some of the changes to the NextSettingsForm are not related to D11 compatibility. Don't doubt that these could be worthwhile changes, but keeping them in a separate PR will hopefully make it easier for us to get important things Drupal version compatibility through more quickly.
I confirmed that the module tests are passing locally, and also did some manual e2e testing.
There are still sone deprecation warnings thrown by the module tests, but I'll create a follow up PR for that so we can merge this.
backlineint
dismissed
JohnAlbin’s stale review
This feedback has been resolved.
|
This has been released as 2.0.0-beta1: https://www.drupal.org/project/next/releases/2.0.0-beta1 |
This pull request is for: (mark with an "x")
examples/*modules/nextpackages/next-drupalstarters/basic-starterstarters/graphql-starterstarters/pages-starterGitHub Issue: #
Please add a link to the GitHub issue: #777
where this problem is discussed.
Getting ready for this
Code changes need test coverage. If you don't know
how to make tests, check this box to ask for help.
Describe your changes
A clear and concise description of what the request is.
If applicable, add screenshots to help explain your issue.