Automated Drupal 10 compatibility fixes

Theme is compatible with D10 now.

Kindly review patch.

I will review this patch

patch is applied successfully and theme is compatible with drupal10 and is able to install in drupal 10
attached screenshot
so we can move status to RTBC

@sharayurajput Thanks for reviewing. Did you review the theme in Drupal 10 by clicking around the site? We need to make sure the theme is working as expected and not just that it installs properly. Thanks.

Refactoring provided uses services introduced in later versions. That means it needs to be introduced conditionally to avoid run-time errors.

The alternative would be to release a new major with changed requirements. file_url_generator introduced in 9.3.

No idea whether this bug was report/fixed with the bot.

Adding a patch to conditionally check if the service exists.

One more thing I am not sure about is that, this theme makes use of seven as base theme, now that sever has been removed from core should we add seven (contrib theme) as a requirement and composer json?

The code should work on D8, D9 & D10. Looks like you removed the code for D8, <D9.3 and only took care of >=D9.3 and D10.

Working during porting day.

Tagging for visibility.

I would recommend removing theme seven as the base theme and probably switched to Claro for full support in Drupal 9.5 || 10

Theme Seven have been deprecated as of Drupal 9.5

Did a quick test looks like some minor css changes/updates will be needed to retain same look and feel as well as css library overrides for Claro css/components/messages.css

Seven is being minimally maintained as a contrib theme for Drupal 10:

https://www.drupal.org/project/seven

I created a simple PR if anyone wants to have a go at testing.

The merge request doesn't seem to address the project with inheriting from the seven theme. Either this module needs to rely on the contrib seven theme in its composer.json or needs to be converted to use a different base theme for 10.

Could you remove your JetBrains settings from your merge as well?

Since Adminimal is based on Seven, which is marked as deprecated in Drupal 9.5 and will be removed finally in Drupal 10, I fear that most will decide to switch to Claro (core) in November 2023. Which I would find a shame, as Claro is from a typographic point of view a step backwards from Adminimal. I would like to be able to continue using Adminimal, but due to lack of insight I can't estimate the effort to switch to another base theme.

Probably it would be the least effort to rebase Adminimal on the Seven (contrib) theme, as ameymudras suggested in #11. What steps would need to be taken to achieve this? Maybe someone can put this together who has a deeper insight? Probably a new issue would have to be opened for this as well.

I would avoid adding a hard dependency on seven in contrib to the `composer.json` file. If you are on D9, this works with core seven, if you are on D10, it will work with contrib seven. We have `base theme: seven` in the info.yml file. If you want to make it a hard requirement, this module will have to create a new major version just for D10.

I agree with others that keeping seven as the base theme is probably the best way to go. Rewriting it to be based off claro will take some time so it's less likely to happen.

@Jaesin: My concern is that if someone is new from a D10 installation and wants to give Adminimal a try, they might fail because Seven is not present. So either it has to be communicated clearly in the module description that Seven is required or a new version for D10 with hard dependency on Seven contrib has to be created.

@kreatIL, I understand. Let's see what direction the maintainers want to take it.

I have applied the patch from MR6 and for the most part, the upgrade status module is happy. It is dubious about the appearance of the file_create_url function, but the way @jrglasgow is using it seem merely to be for backwards-compatibility, not the de facto method of file upload. I'm inclined to say this is probably ready to go unless there is more discussion to be had in regards to inheritance.

@loopy1492,
Actually the file_create_url() work you are talking about was done by Jaesin in this patch and it could be removed if we wanted to limit the compatibility to ^9.3 || ^ 10, since 8.x and 9.3.x are no longer supported versions of Drupal core I have no problem with that.

setting needs work to remove the file_url code mentioned

I added the #8's suggestion. Please review.

Looks good. Can we remove Drupal core from the composer.json file please? It's already listed in the .info file which is all Drupal cares about. Once Drupal 9.3 is end of life, we will need to edit the composer.json file to remove 9.3 core anyway to prevent security warnings and no one should be installing Drupal core by requiring the theme first, so we shouldn't encourage composer to install Drupal core that way.

@Greg Boggs I was looking at composer.json and I saw the seven dependency. What about it? It was removed from Drupal 10.

Looks good.

As far as the composer change suggested in #31, I think its fine the way it is. Its not a security problem to have an older version of the dependency in the list and listing your dependencies is how composer.json is suppose to work. I think the packaging script would generally take care of it but being explicit shouldn't be a bad thing.

re #32 The seven dependency is handled by contrib in 10 which is what the composer.json file is saying. https://drupal.org/project/seven

Adjusted a slight typo in core version requirement.

Was: core_version_requirement: ^9.3 || ^ 10

Now: core_version_requirement: ^9.3 || ^10

The explicit requirement for an old version of Drupal is a security vulnerability because that allows Drupal 9.0 to be installed which is end of life software. If you're going to specify drupal versions in composer.json, then you must keep that updated with each version of Drupal 9 that expires, so you're going to be committing to the file once per minor release of Core. If you leave that out, then you can not update the composer file every couple of months.

And no one should ever install Drupal core by doing composer require drupal/adminimal and we should even make sure that doesn't work incase someone makes that mistake.

https://git.drupalcode.org/project/adminimal_theme/-/merge_requests/6/di...

And, most importantly, it's super unlikely that someone is going to keep every contrib module updated for every minor release of core. So, the fix here is just not to mention core in Composer.json because it adds no value while giving us maintenance work for every contrib module for every core release.

Removing core requirement from composer.json, per #36.

The core requirement isn't _actually_ a security risk for 2 reasons.

1. Drupal 9 is explicitly supported for a while still
2. After it is no longer supported, it may still be supported by LTS vendors like 8 currently is for some people.
3. The || means there isn't an explicit _requirement, only noting it works with it.

That said, the change in #37 doesn't really matter because Drupal.org packaging will inject the core_version_requirements from the info into the composer file so its effectively the same now as it was before the change.

Still looks good to go.

Whatever the case, I'm glad for projects that are setting the core version requirements to some version of 9 in addition to 10. In preparation for the d10 upgrade, we are running through a d10 readiness story for ALL of our sites before actually making the move to d10. We cannot actually upgrade a few modules yet because the maintainers have decided to remove d9 from the picture entirely which is infuriating.

Hey Guys.
Proposed automated patch didn't completely covered all the theme compatibility. So I created my own one and would like to share it with you.
Good luck!

FWIW: We're using the latest MR in a Drupal 9.5.9 website with the Seven theme from contrib. Works as expected.

Patch in #40 isn't any different then the MR. Can we get it commit and a new D10 compat release tagged please? RTBC++

:pray:

Patch and MR are different actually.

MR includes changes for composer.json

  "require": {
      "drupal/seven": "~1.0"
  }

Instead, the patch contains some unrequired changes.

+1 RTBC for MR anyway.

Can someone merge the changes and make a version for Drupal 10?

Merged to 1.x-dev