- Issue created by @quietone
- @quietone opened merge request.
- πΊπΈUnited States smustgrave
Do we want to open a follow up for the one thread?
Rest seems fine
The Needs Review Queue Bot β tested this issue. It fails the Drupal core commit checks. 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.
- π³πΏNew Zealand quietone
Another rebase, with a conflict that needs review.
- π¬π§United Kingdom catch
It's not good that phpcs is forcing us to ignore this rule when the documentation is there, but when there is another phpcs rule in place that breaks this rule. To me this feels worse than not having the rule enabled at all to be honest.
- π³πΏNew Zealand quietone
Follow up made π Fix unneeded Drupal.Commenting.FunctionComment.Missing Active
The MR is what it was when catch commented. I have commented, made an issue in Coder and a follow up for core, so I think it is OK to restore the RTBC.
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.
- Status changed to Needs work
22 days ago 7:01pm 10 June 2025 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.
- π³πΏNew Zealand quietone
Rebased. There were conflicts due to the update of Coder, which fixed a problem with phpcs:ignore lines such that those lines could be removed. Linting passing so restoring RTBC.
- πΊπΈUnited States xjm
I read all the methods to confirm that the added one-line summaries matched what was actually happening, and looked up the parent class or interface for everything with
{@inheritdoc}
to confirm the docblock described what was happening in the method including signature, return value, etc.I posted one suggestion to fix a grammatical error in a docblock that's being updated to differentiate it from an added docblock. I also suggest doing the GitLab contrib search to check for usages of the mentioned method to decide whether we want a followup to deprecate it or not.
Marking NR to make the status clear, but given that the tasks are just reviewing/applying a suggestion and doing a contrib search, the changes do not need a separate reviewer and author to be re-RTBCed.
- πΊπΈUnited States xjm
Actually @quietone in the linked issue, @klausi claims that the issue will be solved if we upgrade to the latest version of coder. 11.x now is on the latest version of coder. Can we therefore remove these other additional ignores?
- πΊπΈUnited States smustgrave
Believe feedback has been addressed, left the 1 thread open jsut in case
Saved credit for xjm and catch for the MR reviews.
- πΊπΈUnited States xjm
Saving credit for @smustgrave for issue management, since this has gotten complicated.