Inject entity.repository service instead of using \Drupal::service('entity.repository') in non-tests

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Fixed CCF as per #3,

Hi @_pratik_ & @harivansh,
Please do assign to yourself first if you have a quick solution.

I've added a new patch#9 please review.

Hi,
Added a patch#12 wait if test failed then assign to yourself.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Updating title, tags and version number based on recent announcement at https://www.drupal.org/about/core/blog/new-drupal-core-branching-scheme-... →

I will see it.

Hi I just rerolled please revise.

NW for custom commands failed in #18

And this should be using an MR as well.

samit.310@gmail.com → made their first commit to this issue’s fork.

Believe we can use constructor property promotion here.

dimitriskr → changed the visibility of the branch 11.x to hidden.

Left some comments and questions on the MR.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Can deprecations be updated for 11.3

Needs work based on #34

Have updated deprecations to 11.3 based on #34, please review it.

Thanks!

Given the number of changes to constructors that are going on, I'm going to suggest that we implement the proposal in 🌱 [policy, no patch] Allow docblock to be removed when modifying constructor params Active and delete their docblocks. This is especially true for TaxonomyIndexTid. Even if someone feels we shouldn't delete docblocks until the policy is official, we definitely shouldn't be adding one to the new EntityReferenceFormatterBase::__construct().

@dcam I agree on the policy suggestion, but phpcs complains on the pipeline, that's why I had to add them. We need to change our coding standards too.
I suggest moving it back to NR unless this policy is going to be enforced really soon.

@dimitriskr I checked all of the pipelines for this issue to see what you're describing. I only found this one that had a PHPCS failure related to the docblocks: https://git.drupalcode.org/issue/drupal-3344389/-/jobs/3308151. This particular job failed because there was an existing docblock that was missing a parameter.

My suggestion is to delete the constructor docblocks entirely instead of adding the new parameters. The rule to allow constructors without docblocks is part of the Drupal PHPCS configuration. I've created a bunch of constructors without docblocks in my own work. But for the sake of due diligence I tried deleting some of the docblocks in your MR and running PHPCS on the results locally. There were no errors. So you might give it a try.

For what it's worth @catch OK'ed deleting the docblocks even though the policy on issue scope isn't official. Apparently it's been done in a number of issues lately.

That's true, checked locally too.
So if the docs exist and it's wrong, phpcs complains. If it doesn't exist, all is well! I can clean this up

Also, we're not even discussing adding @inheritDoc, right? It's about completely removing the docblocks

Also, we're not even discussing adding @inheritDoc, right? It's about completely removing the docblocks

Yes, that's correct!

docblocks per #3419960-9: [policy, no patch] Allow docblock to be removed when modifying constructor params → removed

I found a couple of classes where it seemed like the new parameters need to be nullable. If I'm wrong, then let me know.

The lack of code style change consistency bothered me. I left suggestions for a few things. Aside from those, I notice one class had its constructor parameters broken up onto multiple lines. Personally, I'm OK with that. Giant in-line constructor definitions are a pain to read, edit, and review. But the MR only does it for one of them. I'd do it for all or none. Finally, one class had its parameters changed to use property promotion. I think this is OK, but not 100% sure because that could be out-of-scope. But again, this was only done for a single class.

Class parameters to property promotion (split into multiple lines was added by me) suggested by smustgrave to be in-scope here

Is there a general plan in core of switching classes to use property promotion/multi-line? I can switch all of these here to it, but I don't want to interfere if there's something existing already.

Class parameters to property promotion (split into multiple lines was added by me) suggested by smustgrave to be in-scope here

@smustgrave would know better than I would.

Is there a general plan in core of switching classes to use property promotion/multi-line?

There's 📌 Use PHP 8 constructor property promotion for existing code Needs work , but I scanned the current MR to see if it's editing any of these same classes and didn't notice any. That's a little weird, but I think you're OK to proceed.