Skip to content
This repository was archived by the owner on Aug 22, 2023. It is now read-only.

Remove Query\Builder::__constructor overload #26

Closed
wants to merge 1 commit into from

Conversation

GromNaN
Copy link
Owner

@GromNaN GromNaN commented Aug 21, 2023

The constructor don't need to be overloaded to set the default grammar. It is read from the connection by default.

https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Query/Builder.php#L246

@GromNaN GromNaN requested review from jmikola and alcaeus August 21, 2023 11:51
@GromNaN GromNaN force-pushed the query-builder-constructor branch from dc0ef90 to 9ba9607 Compare August 21, 2023 11:51
@@ -758,16 +748,16 @@ public function lists($column, $key = null)
/**
* @inheritdoc
*/
public function raw($expression = null)
public function raw($value = null)
Copy link
Owner Author

Choose a reason for hiding this comment

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

Get the argument name from the parent method.

@GromNaN GromNaN force-pushed the query-builder-constructor branch from 9ba9607 to 027166e Compare August 21, 2023 12:22
Copy link
Collaborator

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

LGTM assuming no behavioral changes (see question).

@@ -164,10 +164,10 @@ public function chunkById($count, callable $callback, $column = '_id', $alias =
/**
* @inheritdoc
*/
public function raw($expression = null)
public function raw($value = null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@@ -856,7 +846,7 @@ public function drop($columns)
*/
public function newQuery()
{
return new self($this->connection, $this->processor);
return new self($this->connection, null, $this->processor);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is a null second argument equivalent to constructing a new Grammar object (perhaps by way of calling Connection::getQueryGrammar()), as was done in the original overridden constructor (before the changes in this PR)?

Just want to confirm there's no behavioral change here.

@GromNaN
Copy link
Owner Author

GromNaN commented Aug 22, 2023

Moved to mongodb/laravel-mongodb#2570

@GromNaN GromNaN closed this Aug 22, 2023
@GromNaN GromNaN deleted the query-builder-constructor branch August 22, 2023 09:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants