Starting with core 8.7.7 a new 'core_version_requirement' key can be added to a project's .info.yml file. This change record has all the details.

At the time of writing, Scheduler will only be able to have core_version_requirement: ^8 but we could add this now, so that it is ready in the lead-up to Drupal 9 and our work on #3042677: Drupal 9 Deprecated Code Report for Scheduler and #3035104: [meta] Remove deprecated code flagged by @trigger_error messages for Drupal 9 compatibility

CommentFileSizeAuthor
#21 3079913-21.patch1.01 KBchr.fritsch
#11 3079913-11.patch1.97 KBchr.fritsch
#7 3079913-7.patch986 byteschr.fritsch
#5 3079913-5.patch2.11 KBchr.fritsch
#2 updated-core-version-requirement-in-info-file-3079913-2.patch392 bytessahana _n
#4 3079913-4.patch2.11 KBchr.fritsch

Comments

jonathan1055 created an issue. See original summary.

sahana _n’s picture

sahana _n commented
Status: Active » Needs review
StatusFileSize
new updated-core-version-requirement-in-info-file-3079913-2.patch392 bytes

Added New core version requirement key in info.yml file, please review

jonathan1055’s picture

jonathan1055 commented
Related issues: +#3042677: Drupal 9 Deprecated Code Report for Scheduler, +#3035104: [meta] Remove deprecated code flagged by @trigger_error messages for Drupal 9 compatibility

Hi Sahana_N,
Thanks for this. However I don't know if we can add core_version_requirement: ^8 || ^9 yet while the module still has code which is probably going to be removed in D9.

I know that #3035104: [meta] Remove deprecated code flagged by @trigger_error messages for Drupal 9 compatibility shows we have no deprecated code, but there are still things to do on #3042677: Drupal 9 Deprecated Code Report for Scheduler before we can say we are ready for Drupal9

Or maybe I have mis-understood the meaning of the new 'core_version_requirement' feature.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new 3079913-4.patch2.11 KB

Let's add the core_version_requirement key to all info files.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new 3079913-5.patch2.11 KB

Argg...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented 
+++ b/tests/modules/scheduler_access_test/scheduler_access_test.info.yml
@@ -2,6 +2,6 @@ name: 'Scheduler Node Access Test'
-core: 8.x
+core_version_requirement: ^8.7.7 || ^9

+++ b/tests/modules/scheduler_api_test/scheduler_api_test.info.yml
@@ -2,6 +2,6 @@ name: 'Scheduler API Test'
-core: 8.x
+core_version_requirement: ^8.7.7 || ^9

Test modules don't need a core or core_version_requirement since #3096609: Allow contrib test modules to not need a core or core_version_requirement key landed in 8.8.x.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new 3079913-7.patch986 bytes

Ok, I removed the changes in the test modules and kept the version constraints for scheduler and scheduler_rules_integration

jonathan1055’s picture

jonathan1055 commented

Thanks @alexpott.

@chr.fritshc, your patch in #5 had ^8.7.7 || ^9 but in #7 you have changed it to ^8 || ^9. Please can you explain the reasoning/benefit of one vs the other? Thanks.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@chr.fritsch you can remove the core constraint from the test modules. This might affect 8.7.x testing... but /shrug not sure that matters too much it's in security support only.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

@jonathan1055 With #5 the minimum required version of core is 8.7.7, with #7 you can still use core 8.5 like it is currently.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new 3079913-11.patch1.97 KB

Let's try what @alexpott suggested

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

So, I would use #7 for now. We can remove the core key later from the test modules when 8.7 is not longer supported at all.

jonathan1055’s picture

jonathan1055 commented

Thanks for the answer in #10 - that is what I thought it was, but good to have it confirmed.

Yes I agree we'll use the version in #7 and keep the 8.7 tests running. Easy to change that when 8.8 is the lowest Core version.

One point though - I am sure some time previoously when testing this manually I had an error saying words to the effect "You should have 'core' or 'core_version_requirement' but not both". Not sure where that was created. It is not via coding standards as that output is clean. Also it does not seem to affect d.o. testing.

jonathan1055’s picture

jonathan1055 commented

... or as there is currently a restriction on minimum core 8.5 should we use ^8.5 || ^9 ?

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

I think you get that message when you have something like

 core: 8.x
 core_version_requirement: ^8.7.7 || ^9

but not when you have 

 core: 8.x
 core_version_requirement: ^8 || ^9
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Yeah you need to start from ^8.7.7 - cause that's when core_version_requirement was added. Imo that's fine here. I'd drop 8.7 testing on https://www.drupal.org/node/3292/qa rather than having to do things multiple times. Cause 8.7.x is not going to change. But hey that's me. If you don't want to do that you need to do #5 so you can test on D9.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

Could we get this done, please?

jonathan1055’s picture

jonathan1055 commented

@chr.fritsch
I just re-tested your patch #11 and it does not work in core 8.7. So can we go with patch #7? Would this satisfy your need to install and test at core 9.x?

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

Yes, you can go with #7

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
StatusFileSize
new 3079913-21.patch1.01 KB

Damn, now we missed the testing modules...

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

With #21 applied, also the scheduler_content_moderation_integrations tests for D9 get green.
See https://github.com/thunder/scheduler_content_moderation_integration/runs...

jonathan1055’s picture

jonathan1055 commented

Damn, now we missed the testing modules...

Yes that was by design, see #6, #7, #12, #18, #19
But I can add them back with patch #21 if we need that.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

Yes, please commit #21

jonathan1055’s picture

jonathan1055 commented
Status: Needs review » Fixed

Done.
Now that you have abandoned Travis testing on SCMI it means that you are not running the Scheduler tests alongside SCMI patches. This has proved useful in the past, when SCMI changes worked in your tests but failed in the Scheduler main tests. Are there any plans to add Scheduler's main tests to your new testing set-up?

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

Thank you.

There are no patches anymore.

And running the scheduler module tests with the SCMI module laying next to it doesn't change anything, because all the scheduler tests don't enable SCMI.

Another question: Are you planning to create a new release?

jonathan1055’s picture

jonathan1055 commented

Sorry for the second commit. I just changed the commit message, but could not rebase the branch cleanly for d.o. (maybe it can be done but I was not doing it right) so had to merge, which produced the odd repeated commit, but did have the altered text in the message.

jonathan1055’s picture

jonathan1055 commented

Status: Fixed » Closed (fixed)

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

jonathan1055’s picture

jonathan1055 commented
Parent issue: » #3136831: [meta] Drupal9 Readiness and compatibility - Scheduler