Drupal 10 compatibility

Re-roll patch for Drupal version 10.1.x

I reviewed and verify patch with 8.x.1.x version with Drupal 10 there is non ignorable issues found. Drupal 10 compatibility issues fixed.

I reviewed and verify patch with 8.x.1.x version with Drupal 10 there is non ignorable issues found. Drupal 10 compatibility issues fixed.

I tested patch #6 in drupal 10.0.8 and it looks good to me. Module is made d10 compatible with this patch.

Moving this to RTBC.

Thanks for your work on making the module D10 ready.
Can you provide a new 1.x-dev release please, since the issue is marked as being RTBC?

Thanks in advance,
SteffenR

I've created a Merge Request for the maintainers to merge and for us to use in our composer.json file.

I have no idea why, but patch #12 does not deploy against the dev branch. Here's my revised version:

Darn it, uploaded the wrong file...

Sorry folks, my brain is broken on Friday. Let's try this one last time.

Sorry for all the spam folks.

I'm a bit stumped as to why #12 isn't applying but #18 is. Someone better check this one last time.

I moved this out of "Reviewed and Tested" back to "Needs Review". Something's not quite right with #12.

@TrevorBradley I'm not sure what you tried to accomplish with your patches, but since a main aspect is to introduce the composer.json file into the module, you may use the merge request !2 in your projects composer.json until the maintainers create a new release including the D10 adoptions from that merge request.

You may want to read https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →

Merge request !2 works fine.
Switching to RTBTC

@guedressel Is not recommended to use the MR as a patch because this can be changed by anyone and it's a security risk. If you find an issue you with no patch you should create the patch, basically copying the patch in the MR to a patch file and upload it. This will help to keep the repos safe until the patch is committed.

@AlvaroDeMendoza Simply using a patch file won't work if you try to upgrade your site using composer. Composer reads the module dependencies from the composer.json shipped with the module and would still get the old information from the repository.
Example: running composer why-not drupal/core10.1 with the latest development version of system_stream_wrapper installed will result in drupal/system_stream_wrapper 1.x-dev requires drupal/core (^8 || ^9) even if you apply the patch using composer.
Using the branch (issue fork) of the merge request bypasses the problem, since composer directly uses this branch and gets the correct module dependencies.
If you worry about someone randomly changing the code within the merge request you are able to lock the module in your composer.json on a specific commit (see https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → linked by @guedressel).

RTBC+1 btw

Thank you @stBorchert. That's better than using drupal linient.

@ TrevorBradley the patch struggle is real ;-)

For anyone wondering, patch #12 works against version 1.0.0-alpha3 (that is not latest dev, but the alpha3 version). I, for one, prefer alpha over dev.

RTBC +1

Hey folks, thanks for the efforts on this! I gave it a review. I'd like to sync with Dave Reid too, but he's unavailable right now. Regardless, let's aim to get this committed by the end of the week.

I left a few review notes on the MR. Nothing major, this is pretty close to done.

A total aside:

basically copying the patch in the MR to a patch file and upload it. This will help to keep the repos safe until the patch is committed.

Actually, _technically_ patch files are vulnerable to code swaps too. A regular user can't do it, but there's no authenticity or integrity checks beyond what HTTPS provides in transport. At Lullabot, we've standardized on always committing a copy of a patch to the repository and never referencing them with `https://...`: https://architecture.lullabot.com/adr/20220429-composer-patch-files/

I've updated from my review notes earlier. I also fixed up tests and got them running on GitLab: https://git.drupalcode.org/project/system_stream_wrapper/-/pipelines/51933. I've left the old test bot in place for now.

My plan is to merge this Monday, and switch over to a semantic module version release at the same time, tag a 1.0, and opt in to security coverage.

Thanks everyone for the initial work on this. The MR is committed, tests are passing, and I tagged a 2.0.0 release to get us into semver.

https://www.drupal.org/project/system_stream_wrapper/releases/2.0.0 →

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