Skip to content

Conversation

@LordSimal
Copy link
Contributor

Refs: #310

This contains all the method renames, which can't be simply changed to the non-underscore-prefixed version.

Therefore please feel free to add reviews for new method names.

The doX schema for all the sub-method naming feels right to me, but maybe someone has a better idea 😁

],
'Cake\ORM\Query\SelectQuery' => [
'_decorateResults' => 'ormDecorateResults',
'_execute' => 'ormExecute',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added the orm prefix since both those methods exist in the \Cake\Database\Query\SelectQuery as well.

@LordSimal
Copy link
Contributor Author

The Cake\Http\Client\Response needs to be tackled separately since there are several public methods which just call a prefixed version of itself and return it. This doesn't make any sense to me to be honest, because if someone wants to overwrite that method, they can do it with either the protected method and the public method.

E.g.

public function getJson(): mixed
{
    return $this->_getJson();
}

@ADmad
Copy link
Member

ADmad commented May 31, 2025

The Cake\Http\Client\Response needs to be tackled separately since there are several public methods which just call a prefixed version of itself and return it.

We can just move the code inside the non prefixed methods and get rid of the prefixed ones for these.

@LordSimal
Copy link
Contributor Author

We can just move the code inside the non prefixed methods and get rid of the prefixed ones for these.

yes, thats what I like to do as well.

@LordSimal LordSimal marked this pull request as ready for review June 4, 2025 16:08
@LordSimal LordSimal merged commit a248b02 into 6.x Jun 4, 2025
3 checks passed
@LordSimal LordSimal deleted the 6.x-rename-conflicting-underscored-methods branch June 4, 2025 16:08
LordSimal added a commit that referenced this pull request Jan 6, 2026
* rename conflicting underscored methods

* adjust naming according to feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants