Problem/Motivation

Add Drupal 9 compatibility, follow up from https://www.drupal.org/project/social_pwa/issues/3255925

Issue fork social_pwa-3256100

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

nkoporec created an issue. See original summary.

nkoporec opened merge request !4

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work
Related issues: +#3218432: ActivitySend plugin doesn't support injected services

For some of the changes, there is already an issue: #3218432: ActivitySend plugin doesn't support injected services. Those changes should be removed from the changes proposed here.

basvredeling’s picture

basvredeling
Primary language Dutch
Location Amsterdam
commented

Patch looks pretty solid to me. @apaderno are you talking about the rename of the create() method when saying there is already an issue for parts of the patch?

Since https://www.drupal.org/project/social/issues/3218415 the main method name has been changed to process(). That change was part of the Drupal 9 (open social 11) release. So while technically not a D9 incompatibility, it is part of the D9 updates necessary to get this working with the dependencies.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

@basvredeling I was saying there is already an issue for some of the changes done in this issue. Each issue should be for different changes.

As for the method rename, it doesn't apply to the 10.3.x branch, which means it cannot be applied to this project too. To be applied, this project should require the Open Social distribution 11.x.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Drupal 9 compatibility » Make the modules compatible with the 11.0.x branch of the Open Social distribution

I changed the title, since the patch doesn't make the project compatible with Drupal 9, but with the 11.0.x branch of the Open Social distribution from which the project depends.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented

@apaderno I added Open Social 11 as a dependency. When this gets merged the maintainer can then either tag a new version that will be only OS 11 compatible or branch out the D8 version of this module. I think this is the most straightforward way to do it, I also left the create() changes as it is necessary to have it, otherwise, the module will not install and therefore can not even be tested.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs work » Needs review
nachosalvador’s picture

nachosalvador commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Reviewed & tested by the community » Needs review

After talking with one of the maintainers, we decided to keep the OS 10 compatibility, so I have adjusted the code so it is now compatible with both versions.

nachosalvador’s picture

nachosalvador commented
Status: Needs review » Reviewed & tested by the community

Looks good

  • nkoporec authored 977cb65 on 2.0.x 
    Issue #3256100: Drupal 9 compatibility
robertragas’s picture

robertragas commented
Status: Reviewed & tested by the community » Fixed

Thanks for the work guys, merged it and released it in the 1.4 branch.

Status: Fixed » Closed (fixed)

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