- Issue created by @quietone
- Merge request !7724Resolve #3443435 "Use drupal.namingconventions.validfunctionname in" β (Open) created by quietone
- Status changed to Needs review
4 months ago 9:23am 26 April 2024 - Status changed to RTBC
4 months ago 1:20pm 26 April 2024 - πΊπΈUnited States smustgrave
Kinda same as the other one I reviewed, leaning on the tests are green and the phpcs rule catching anything missed. The changes themselves make sense though.
- Status changed to Needs review
4 months ago 2:20pm 27 April 2024 - π¬π§United Kingdom alexpott πͺπΊπ
Are we sure about this change? There is a PHP class \DOMXPath. I'm not sure that this is a good rule to implement in core for two reasons.
1. PHP does not care about case for method names. If a class has a method getXPath() and I call getxpath() it'll work just fine.
2. We might be forcing developers to make bad choices for no reason. For example, say you work for the BBC and you implementing a method getBBCListing() - there is probably a corporate standard that says BBC must always be capitalised but our coding standards prevent that. I'm not saying I particularly like the method name getBBCListing() but I'm not sure that our coding standards should have an opinion. Also if you are extending PHP's classes you have things like \DOMNode::C14N and \DOMNode::C14NFile. - πΊπΈUnited States smustgrave
Postponed the other 2 so main conversation can happen here.
- Status changed to Needs work
4 months ago 9:17pm 30 April 2024 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.