\Drupal calls should be avoided in classes, use dependency injection instead in CronAccessCheck.php

+++ b/core/modules/system/system.services.yml
@@ -1,6 +1,7 @@
+    arguments: ['@logger.channel.default', '@state']

This should be @logger.channel.cron.

I've re-rolled patch for 9.1

sorry please avoid the above patch.

re-rolled patch for 9.1

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.

Seems now that the service requires arguments it will need a BC layer.
In the constructor they should default to NULL with a trigger_error that it will be required in D11
Which then requires tests for the message.

Change record will also have to be added announcing this service arguments are required.

Re-roll the Patch For 10.1x Branch.

Updating to a MR against 11.x and incorporating the BC trigger_error warnings per #21 and https://www.drupal.org/about/core/policies/core-change-policies/drupal-d....

Clarification on tests; as of 2023-12-15, https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... states

Tests are not required where the test would only be asserting that a deprecation message is triggered

The suggested tests would fall under only asserting a deprecation message? Is it required?

As I understand it, the change record is still necessary. Just wanted to confirm the test requirement because it seems contradictory.

So kinda,

Relooking at this I'm wondering if we have to do the backwards compatibility dance. If a contrib module could extend this class they won't have the new services we are passing into it so would need some trigger_errors

And those would need simple test cases for the exception message.

Gotcha, updated the MR with a basic kernel test to assert the deprecation messaging and drafted up a change record.

That looks like it should be enough? Putting back in needs review.

Deprecation additions look good to me.

I'm triaging RTBC issues. There is no issue summary here! The only information is the title.

An issue summary really is needed no matter how simple one may think the issue is. It is what helps us at every stage of an issue to complete it and keep track of details.
--

This looks like it should be part of a meta. I say that because we normally do not make such changes by file, they are scoped by the type of change. In this case there would be an issue for changing all instances of logger and all instances of state.

After a very brief search I found #2729597: [meta] Replace \Drupal with injected services where appropriate in core where there are existing issues for logger and state. The work here needs to move to #2981326: Replace non-test usages of \Drupal::logger() with IoC injection and #3123228: Replace non-test usages of \Drupal::state() with IoC injection.

If you are unsure about scope, you can read about Drupal standard practice for how to scope issue. And there is always the option to ask in Drupal slack.