[meta] Add support for Drupal 10

The MR and patch are not in sync, it looks like the patch is the most up to date, can we close the MR to avoid further confusion?

Fixes #11, I'm still unsure if the failures need the expected values updated or they're highlighting an actual bug.

Also changed some issets to null coalescing.

The last submitted patch, 14: 3288663-14.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

I'm assuming that the expected strings should be updated as we're following the deprecation changes correctly. We also need to fix the return type of ::mimeMailAddress as it can return an array.

#16 seems good but is missing the composer.json

I just saw in the #12 that the composer.json was added but as cited in the #13 we stopped using MR and used only patches. As I can see the composer.json isn't included on this patch

I didn't saw it before and I reported it separated at ✨ Add a composer.json Fixed

I'm including this as a "related" and I think we can merge it separated

I really do not like automated kitchen-sink patches like this, because they're always wrong in the details and because maintainers tend to commit them without questioning, which creates more problems for the future.

In this case, the change:

-core_version_requirement: ^8.8.2 || ^9
+core_version_requirement: ^8.8.2 || ^9 || ^10

is wrong. After this patch, this module will absolutely NOT be compatible with Drupal 8 or Drupal 9.0 or Drupal 9.1.

I would really like to have distinct patches for the several issues we have in this module related to D10 support. Specifically, for features added in Drupal core in D9, or removed from Drupal core in D10, we need to ensure that our dependencies are declared correctly and that we're not (for example) using things that are only available in D10 but claiming we're still D9 compatible.

I'm assuming that the expected strings should be updated as we're following the deprecation changes correctly. We also need to fix the return type of ::mimeMailAddress as it can return an array.

Yes. And the MimeMailFormatHelperTest should also be updated to test the array return type. I would actually like to refactor mimeMailAddress() because, as I have said many times previously, like many other parts of this module it's a really bad mess that tries to do too many things in one function. Having a complete set of tests allows us to do this refactoring without breaking anything.

Also, why does the patch inject the Request into the MimeMail plugin? It's not being used anywhere ...

As a first step here, I'd like to see a new issue for the core change https://www.drupal.org/node/3207439 which includes the test string changes and the return type change/additional test as discussed above. This also requires increasing the Drupal core version to 9.2 and dropping support entirely for Drupal 8.

Then we can proceed to adding return types to all the test methods (not just the setUp() methods). Again, in a separate issue.

Then we can deal with the deprecation of file_create_url(), which also needs another change in Drupal core version supported.

The absolute last step should be changing the core version requirement to declare that this module works on D10, and we should do that only after the tests run green in Drupal 10. A new release with the D10 support can also be rolled at that point. We can do that here in this issue, but only after all the other things are fixed.

The child issues have been fixed and committed, so let's see if the tests pass when we up the core version requirement and test against D10.

Committed #22 and closing this issue because the tests now run on Drupal 10.

If further D10-specific issues arise, please open a new issue for each.

Thanks @TR - what's the plan for releasing this? Will there be a new alpha/beta on 1.x? Or a new branch?

A new release would be nice.. And considering this never left alpha, I'd think a new branch is not needed. Maybe one last alpha to iron out the D10 kinks it may still have, but then maybe a beta or rc release?

New release on the 8.x-1.x branch first, then open a new 2.0.x branch to make the API changes and refactoring that are necessary to keep this project moving forward. I have a few clean-up issues to open and commit before the release, but that shouldn't take more than a few days.

Maybe one last alpha to iron out the D10 kinks it may still have, but then maybe a beta or rc release?

There are only two issues in this module which are marked as beta blockers - I think I have been pretty conservative in that regard. See https://www.drupal.org/project/issues/search?text=&projects=Mime+Mail&as... →

But they have been marked that way for almost THREE YEARS, and I haven't had any participation from the community to get those finished.

And there are also some fundamental features that have not been fully ported from D7 yet - image attachments don't fully work, for example.

I believe in truth in advertising, and if a module is labeled 1.0 I expect it to perform its basic function. If it doesn't IMO it should be labeled an alpha or beta. I recognize that others don't see it that way, but I am not able by myself to deal with all the things that need to be fixed, and I am unwilling to deal with all the new support issues that WILL be raised by a 1.0 release that I KNOW does not work correctly.

So I will probably make another alpha release at this point then branch off a 2.0.x and work on re-writing all the legacy code that has proven to be far more work to fix than it would be to re-write.

@TR You have 24K installs of this module in Drupal 8/9 sites, I don't think that anyone actually believes any of the beta blockers to be important, nor do I think that users have skipped on creating issues just because it's an alpha version.

But you're the maintainer, and I do appreciate your work on this important module, so if you don't think it is ready for beta, then it is not ready for beta. End of discussion from my part. Fingers crossed for the alpha5 release being soon, though.

The beta blocker is important to ME because this code desperately needs to be refactored and rewritten, and until we have tests for some of this we can't do that without breaking things and causing major regressions. No one, including me, understands a lot of this legacy code, and writing the tests for the existing methods documents and defines what those methods are intended to do. I have already done a tremendous amount of work towards that, but there are still major parts that are undocumented and untested and not-really working.