Problem/Motivation

API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

The documentation does not mention that there is a key called core_version_requirement and that it is a required field.

Steps to reproduce

Proposed resolution

Document the core_version_requirements key in \Drupal\Core\Extension\InfoParserInterface in the 'Information stored in all .info.yml files:' section.

Remaining tasks

Make a patch
Review
Commit

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3080819

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Vivek Panicker created an issue. See original summary.

Vivek Panicker’s picture

Title: Missing documentation for "core" key » Missing documentation for "core" key in InfoParserInterface file.
Issue summary: View changes

Version: 8.2.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Branches prior to 8.8.x are not supported, and Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Version: 8.9.x-dev » 9.4.x-dev
Issue summary: View changes
Issue tags: -Documentation +Bug Smash Initiative, +Novice

The key 'core' has been deprecated, info.yml files no longer accept 'core: 9.x' and replaced with 'core_version_requirement', New 'core_version_requirement' key in info.yml files for modules, themes and profiles allows Composer semantic version constraints including specifying multiple major versions of core. However, 'core_version_requirement' is not documented on \Drupal\Core\Extension\InfoParserInterface like the other keys, such as 'package'. Therefore updating the IS to add documentation for 'core_version_requirement'. I reckon this is suitable as a Novice issue.

Since this is a documentation only change, when uploading the patch, select 'Do not test' (and save on resources). The tests can be run when this is ready for RTBC.

Edit: fix links

quietone’s picture

Title: Missing documentation for "core" key in InfoParserInterface file. » Missing documentation for "core_version_requirements" key in InfoParserInterface file.

Forgot to update the title.

beatrizrodrigues’s picture

Assigned: Unassigned » beatrizrodrigues
beatrizrodrigues’s picture

Assigned: beatrizrodrigues » Unassigned
Status: Active » Needs review
FileSize
814 bytes

Here's the patch with the documentation for "core_version_requirement". I hope the description fits.

anagomes’s picture

Status: Needs review » Reviewed & tested by the community

The patch in #7 applies correctly. It briefly describes the core_version_requirement key and makes it clear what it means and that it is required in the info.yml file. Looks good to me!

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I have fixed the links in my earlier comment.

@beatrizrodrigues, thanks for making a patch and not running the tests yet.

@anagomes, thanks for your interest. Before a patch can be RTBC it also needs to pass tests. Yes, even for a simple documentation change. There is more detail about the steps in core gate

This should also that core_version_requirement applies to themes and profiles as well. Of course, it didn't help that the links in my comment were broken. They are fixed now.

I also think we should go one step further and show how to add multiple versions. Something like this? Change as you see fit.

core_version_requirement: Specifies the version or versions of Drupal core. When the project is compatible with more that one version separate the versions with "||". For example, for a project compatible with both Drupal 9 and 10 use ^9 || ^10. (Required)

The use of (Required) at the end is not needed, instead optional parameters are identified, as per Making lists in documentation. However, because Required is used elsewhere in this file I think that should stay. If anything, a followup can be made to improve the Interface document by cleaning up the Required and adding complete examples. Although, there may be an issue for that already and I am not able to search right now.

beatrizrodrigues’s picture

Assigned: Unassigned » beatrizrodrigues

thank you for your feedback @quietone

The description gets really better like this, indeed. Thank you for the links and for sharing your knowledge with me. I'll apply the improvements to the documentation and send the patch here. And I will run the tests for it. Thank you again!

beatrizrodrigues’s picture

Assigned: beatrizrodrigues » Unassigned
Status: Needs work » Needs review
FileSize
978 bytes
1.01 KB

I think the description you gave @quietone fits perfectly, so I applied it. Here is the patch where I did the improvement suggested, and a interdiff too. (I searched for the issue about cleaning up the Required but I was not able to find it, i'll keep searching, because I would gladly work on that.) Thanks again.

lucienchalom made their first commit to this issue’s fork.

lucienchalom’s picture

Status: Needs review » Reviewed & tested by the community

The last patch apply cleanly and I liked and agree with the addition of the example.
moving to RTBC.

quietone’s picture

Status: Reviewed & tested by the community » Needs work

@beatrizrodrigues, thanks again.

Oh no, I see a typo.

+++ b/core/lib/Drupal/Core/Extension/InfoParserInterface.php
@@ -20,6 +20,10 @@ interface InfoParserInterface {
+   *   Drupal 9 and 10 use ^9 || ^10.. (Required)

s/../.

beatrizrodrigues’s picture

Status: Needs work » Needs review
FileSize
980 bytes
1.02 KB

Thanks @quietone I hope all the typos were fixed. Here is a patch and a interdiff.

edit: Sorry, I forgot to make the interdiff with .txt extension.

quietone’s picture

Status: Needs review » Needs work

I like the additional comma, it reads better, but there is still an instance of two periods.

+++ b/core/lib/Drupal/Core/Extension/InfoParserInterface.php
@@ -20,6 +20,10 @@ interface InfoParserInterface {
+   *   with both Drupal 9 and 10 uses ^9 || ^10.. (Required)

This should be '.' not '..'.

beatrizrodrigues’s picture

Status: Needs work » Needs review
FileSize
979 bytes
0 bytes

Thanks @quietone, I was missing that extra ".". Here's the new patch and interdiff.

quietone’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Core/Extension/InfoParserInterface.php
@@ -20,6 +20,10 @@ interface InfoParserInterface {
+   *   with both Drupal 9 and 10 uses ^9 || ^10. (Required)

I don't believe I missed another one! 'uses' should be 'use'. Looking back, that change happened in #15 and obviously I was looking at the '..'.

I applied the patch and read the change (slowly) and realized that it would be better with quotes. Since this needs a change for the word 'use' we might as well do this one too. Here is what I suggest:

* with both Drupal 9 and 10 use "^9 || ^10", without quotes. (Required)

beatrizrodrigues’s picture

thanks again @quietone Here is the patch and the interdiff with the changes suggested.

beatrizrodrigues’s picture

Status: Needs work » Needs review
paulocs’s picture

Status: Needs review » Reviewed & tested by the community

I'm not a native English speaker but it looks good to me. It's very clear for me and I don't see any typo.
Moving to RTBC.

quietone’s picture

@beatrizrodrigues, thanks for seeing this through to RTBC.

catch’s picture

Status: Reviewed & tested by the community » Needs work

I think this looks good, but... could we also mention the possibility of using >= here? For example >= 8 to allow compatibility with Drupal 8, 9 and 10 (and later versions) without needing to update each major release.

beatrizrodrigues’s picture

Status: Needs work » Needs review
FileSize
1.08 KB
924 bytes

@catch thanks, I adapt a little bit you suggest. Please, let me know if it fits.

quietone’s picture

Status: Needs review » Needs work

That is interesting. What catch describes is not listed on the change record. Perhaps that change record should be updated?

I tested locally and found that only this form worked, core_version_requirement: ">=8". The quotes are needed. Also, that "<" works as well.

I was thinking something like this which uses examples to show the options.
* - core_version_requirement: Specifies the compatible version or versions of Drupal core. For example, "9.3 || 9.4" means compatibility with Drupal 9.3 and 9.4; ">=9" means compatible with Drupal 9, 10 and later versions, "<=9" mean compatible with Drupal 8 an 9. (Required)

ankithashetty’s picture

Status: Needs work » Needs review
FileSize
960 bytes
1.31 KB

Updated the patch in #24 with the changes suggested in #25, thanks!

ankithashetty’s picture

FileSize
1.31 KB

Sorry, messed up the interdiff file!

hmendes’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

The patch addresses the suggestion from #25 and looks good to me. Changing this to RTBC.

  • catch committed eecfb03 on 10.0.x
    Issue #3080819 by beatrizrodrigues, ankithashetty, lucienchalom,...

  • catch committed bdf5213 on 9.4.x
    Issue #3080819 by beatrizrodrigues, ankithashetty, lucienchalom,...

  • catch committed 20deaf4 on 9.3.x
    Issue #3080819 by beatrizrodrigues, ankithashetty, lucienchalom,...
catch’s picture

Version: 9.4.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

I found a couple of typos in #26 but have fixed them on commit. Also update the change record at https://www.drupal.org/node/3070687

Committed/pushed to 10.0.x, cherry-picked to 9.x and 8.x, thanks!

Status: Fixed » Closed (fixed)

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