- Issue created by @fjgarlin
- 🇩🇪Germany jurgenhaas Gottmadingen
Thanks for starting this follow-up. I'm not sure, though, if such a special treatment is even required. While there is in fact no Drupal 11 alpha yet, we can already "use" Drupal 11 with the dev release. And that's what's happening when testing against the next major. It's like a
composer require drupal/core:^11.x-dev
, no alpha required really.So, with that in mind, there is no reason why contrib modules couldn't already declare Drupal 11 compatibility in their info.yml file.
But I'm not pushing on this, I'm also OK if we keep this "lazy fix" in the template and remove it later. The only potential downside is that then some tests may suddenly fail, that worked OK before, and then the noise might be bigger.
- 🇪🇸Spain fjgarlin
Note that the check is no longer in the "before_script" part, it's in an internal reusable template named
.amend-core-requirements-drupal-11
. - 🇨🇭Switzerland berdir Switzerland
what about adding a warning when this is detected but still do it anyway?
I think it's useful to see the current state without forcing compatibility.
> So, with that in mind, there is no reason why contrib modules couldn't already declare Drupal 11 compatibility in their info.yml file.
There are in fact reasons for that. 11.x is still in development, major changes still happen. The symfony 7 update isn't committed yet, phpunit will be updated to version 10. Those things are quite likely going to break your module. You can't take a compatibility back. once it's out there in a release, composer will install that, possibly breaking sites.
- 🇪🇸Spain fjgarlin
That's a good idea. Just add a warning if the CI job is "auto-fixing" the compatibility in the *.info.yml file.
- 🇨🇭Switzerland berdir Switzerland
I think we could even go as far as making the job fail in that case, since it is marked as being allowed to fail. Then even if the tests are passing, it's helpful in that it's running the tests but it also tells you very clearly that it is not actually "next major" compatible yet.
I think now that it's all part of script, that would be fairly easy. store the output of sed (which I think varies depending on whether or not it did any replacements) in a variable, set that as exit code at the end.
- 🇪🇸Spain fjgarlin
Fully agree with that approach.
I altered a bit the issue description.
- 🇪🇸Spain fjgarlin
The snippet has been refactored a few times, and made more generic here 📌 Update templates so 11.0 is the default/current branch RTBC .
It also shows a diff of what's changed.We are in the same situation now with Drupal 12 and also have the "Upgrade status" job, where it provides fixes for the module.
The only thing that we are not doing is to trigger an error, because in situations like this (Drupal 12 doesn't exist yet), it wouldn't make sense.
Note that we also have the option to have an empty value for "next major", which will not run the variant.All in all, I think that the changes made in several issues made the current approach good enough. I'll close this issue unless we feel very strongly about throwing the error.
- Status changed to Closed: outdated
3 months ago 11:17am 11 September 2024