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

Comments

robertoperuzzo created an issue. See original summary.

robertoperuzzo’s picture

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

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

mrpauldriver commented

Tested okay with php 7.2 and 7.3

Tested with Drupal Check

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
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
StatusFileSize
new 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 Qc, Canada 🇨🇦
commented
StatusFileSize
new admin_toolbar-compatibility_with_D9-3094517-6.patch2.31 KB
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
oknate’s picture

oknate
Location Greater New York City Area
commented
StatusFileSize
new 3094517--interdiff-6-8.txt1.31 KB
new 3094517-8.patch2.52 KB
oknate’s picture

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

romainj commented

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

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented

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

oknate’s picture

oknate
Location Greater New York City Area
commented
Issue summary: View changes
oknate’s picture

oknate
Location Greater New York City Area
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 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
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
commented
Issue tags: +Drupal 9 readiness, +Novice
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: -Drupal 9 readiness +Drupal 9 compatibility
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Status: Needs work » Needs review
StatusFileSize
new admin_toolbar-compatibility_with_D9-3094517-17.patch3.19 KB
berdir’s picture

berdir
Primary language German
Location Switzerland
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 Qc, Canada 🇨🇦
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 🇮🇹
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 Qc, Canada 🇨🇦
commented

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

mondrake’s picture

mondrake
Location 🇮🇹
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
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.