Problem/Motivation

Add core_version_requirement: ^8 || ^9 to admin_toolbar.info.yml to designate that the module is compatible with Drupal 9. See https://www.drupal.org/node/3070687.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#18 admin_toolbar-compatibility_with_D9-3094517-17.patch3.19 KBadriancid
#8 3094517-8.patch2.52 KBoknate
#8 3094517--interdiff-6-8.txt1.31 KBoknate
#6 admin_toolbar-compatibility_with_D9-3094517-6.patch2.31 KBadriancid
#5 admin_toolbar-compatibility_with_D9-3094517-5.patch2.29 KBadriancid
#2 admin_toolbar-compatibility_with_D9-3094517-2.patch289 bytesrobertoperuzzo
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

robertoperuzzo created an issue. See original summary.

robertoperuzzo’s picture

robertoperuzzo
Primary language Italian
Location 🇮🇹 Tezze sul Brenta, VI
CreditAttribution: robertoperuzzo commented
FileSize
admin_toolbar-compatibility_with_D9-3094517-2.patch289 bytes
robertoperuzzo’s picture

robertoperuzzo
Primary language Italian
Location 🇮🇹 Tezze sul Brenta, VI
CreditAttribution: robertoperuzzo commented
Assigned: robertoperuzzo » Unassigned
Status: Active » Needs review
MrPaulDriver’s picture

MrPaulDriver CreditAttribution: MrPaulDriver commented

Tested okay with php 7.2 and 7.3

Tested with Drupal Check

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and at Drupiter commented
Title: Compatibility with Drupal 9 » Add the 'core_version_requirement' key in info.yml and composer.json files
Status: Needs review » Reviewed & tested by the community
FileSize
admin_toolbar-compatibility_with_D9-3094517-5.patch2.29 KB

We will commit this patch when D8.8 will be released.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and at Drupiter commented
FileSize
admin_toolbar-compatibility_with_D9-3094517-6.patch2.31 KB
adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and at Drupiter commented
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
3094517--interdiff-6-8.txt1.31 KB
3094517-8.patch2.52 KB
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Reviewed & tested by the community » Needs work
romainj’s picture

romainj CreditAttribution: romainj as a volunteer commented

When applying the patch #8 I can no longer install the Admin Toolbar Tools module.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and at Drupiter commented

@romainj the patch #8 will only works for Drupal >= 8.8

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented

It looks like we should update our tests to start testing against Drupal 8.8 before this is committed.

We should let people know, if they want to run less than Drupal 8.8, they'll need to restore the 'core: 8.x' part of the info file.

mmjvb’s picture

mmjvb CreditAttribution: mmjvb as a volunteer commented

Suggest to revert #8 just because it restricts usage of the new release to Drupal versions supporting core_version_requirement. My understanding is that was introduced in 8.7.8, with plans to backport.

Why would you want to restrict the new release to those versions of Drupal ?

Apart from the fact it changes the scope of the issue which should be reflected in the summary!

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Yes, not sure why this mixes ^8 and ^8.8, since before it required 8.6 and nothing else was changed it seems like it should be fine to consistently use ^8.7.7 || ^9 now in all .info.yml and composer.json files (not sure if nested composer.json files even work but it's there now, so should go with it).

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Issue tags: +Drupal 9 readiness, +Novice
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Issue tags: -Drupal 9 readiness +Drupal 9 compatibility
adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and at Drupiter commented
Status: Needs work » Needs review
FileSize
admin_toolbar-compatibility_with_D9-3094517-17.patch3.19 KB
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Looks fine to me, not sure why you don't use the identical string in composer.json and info.yml (^9 vs 9.0 and spaces) but this should work fine ;)

  • adriancid committed ee04bc6 on 8.x-2.x 
    Issue #3094517 by adriancid, oknate, robertoperuzzo, Berdir, mmjvb: Add...
adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and at Drupiter commented
Status: Reviewed & tested by the community » Fixed

Thanks @Berdir, I just copy and paste from somewhere with this format, I will put before commit the same format in both place.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented

Hi there, any plan for a point release with this included? Currently 2.0 does not install under D9 dev.

adriancid’s picture

adriancid
Primary language Spanish
Location Montreal, Canada
CreditAttribution: adriancid as a volunteer and at Drupiter commented

@mondrake no fot the moment, you can use the dev version.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented

Alright, thanks!

Status: Fixed » Closed (fixed)

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

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Hook 42 commented

There was a discussion about adding the zero at the end of the versions in the #d9readiness Slack channel during Drupal 9 porting weekend and the consensus was that it's best practice to leave off the unnecessary zero, e.g.

Instead of:

core_version_requirement: ^8.7.7 || ^9.0

you are encouraged to use:

core_version_requirement: ^8.7.7 || ^9

Similar to the change record examples:

https://www.drupal.org/node/3070687

I've reviewed about 50 projects so far and almost all didn't include the extra zero.