Rename RedirectLeadingSlashesSubscriber

Renamed class RedirectLeadingSlashesSubscriber to RedirectSuccessiveSlashesSubscriber

Patch already has the renamed file, it is missing the rename of the old filename to the new one.

Oh really, I missed this, thanks)

Added a new patch as #7 is not getting applied.

+++ b/core/core.services.yml
@@ -1242,7 +1242,7 @@ services:
   redirect_leading_slashes_subscriber:

I guess we should rename the service at the same time.

Event subscribers are not considered API so to me this is safe to do in 10.0.x without providing backward compatibility.

Renamed service as well.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

This will need a change record as the service may in use by others right?

Shouldn't this be deprecated first before replacing?

I could find no uses of the class or service in contrib, and as per the link in #8 event subscribers are not considered API - extending them usually doesn't make much sense, so I don't think we need to spend the effort deprecating this. We can however write a short change notice just in case, that is a simple task that keeps a record of what has happened here.

Added change notice, please, take a look.

Thanks!

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Created MR and all tests are green.
Also, changed the target branch.

Reroll to MR seems fine.

A service machine name is public API; therefore, a CR alone is not sufficient for that change. (The handbook page refers to the class implementation of the event subscriber, which is indeed internal API.) We should deprecate the old service for removal in D11.

We should also provide best-effort BC and deprecation for the class name, which is internal API. Thanks!

My kneejerk about the core service definition machine names might be wrong here; discussing with the other release managers. Hold on changes for now.

I am so sorry that this fell off my radar! I was waiting on feedback from another release manager but then lost track of the issue somehow. Basically everything I said was wrong; the CR is plenty for this because it's an event subscriber, not a normal service. Therefore, even changing the machine name of the service is fine. Per @catch:

Yeah the very worst that could happen is:
Someone is calling it directly (why???)
Someone is decorating it (why???)
It'd be like implementing hook_module_implements_alter() to replace a hook with one that could have just been implemented as an extra hook. Or calling template_preprocess_node() directly. It's not impossible but it's not our fault.

Obviously, the merge request will need to be updated after all this time, but once it's updated and reviewed, the RTBC should be restored if appropriate, without any deprecation or BC layer or anything. Apologies again!

Saving credits.