docs: use modern named arguments syntax

[kaznovac]

use official named arguments syntax in example instead of pre php 8 codestyle for 'named' arguments

[greg0ire]

Good idea. I can find more occurrences of this:

$ rg 'new \w+\(.*\$.*=' docs
docs/en/tutorials/pagination.rst
18:    $paginator = new Paginator($query, $fetchJoinCollection = true);

docs/en/cookbook/aggregate-fields.rst
214:            $account = new Account("123456", $maxCredit = 200);
226:            $account = new Account("123456", $maxCredit = 200);

$ rg '>\w+\([^\)]*\$[^\)]*=' docs
docs/en/reference/dql-doctrine-query-language.rst
1384:          ->setResultCacheLifeTime($seconds = 3600);
1395:    $query->useResultCache(true, $seconds = 3600, 'my_query_result');

Can you please fix those as well?

[kaznovac]

Of course, I'll look into it later

[kaznovac]

@greg0ire done

searched through the rest of documentation, no further instances

---

however:

there is this one, looks a bit like definition of default value:

orm/docs/en/reference/dql-doctrine-query-language.rst

Lines 1460 to 1461 in eafc0c6

	- ``Query::setQueryCacheLifeTime($seconds = 3600)`` - Set lifetime
	of the query caching.

but method definition does not specify the default value:

orm/lib/Doctrine/ORM/Query.php

Line 544 in d81afdb

public function setQueryCacheLifetime($timeToLive): self

how would you like to proceed?
i'd guess this one should also be rewritten as named argument

[greg0ire]

i'd guess this one should also be rewritten as named argument

Yeah but would you then change the Query part to $query? Maybe what's best would be to drop the = 3600 part instead 🤔

[greg0ire]

Please squash your commits together, you shouldn't do one commit per file.

[SenseException]

I'm not sure about this change in the docs. Introducing this would need us to check the docs everytime when we change an internal variable name of an argument when it's a method with just one argument.

The variable $seconds was just there for context. Should the seconds value be used directly?

[SenseException]

This would be lifetime too.

Suggested change

	$query->useResultCache(true, seconds: 3600, 'my_query_result');
	$query->useResultCache(true, lifetime: 3600, 'my_query_result');

[kaznovac]

This method is deprecated since 2.7

should the documentation still promote its use?

[kaznovac]

I've asked as I'd like to remove this reference, or should we fix it now and remove it in the future?

[derrabus]

Yes, we should document the non-deprecated method here.

[kaznovac]

I've asked because method userResultCache is deprecated, and this section is written in a language that kind of 'promotes' it's use. I'd suggest to replace these lines with suggested replacement enableResultCache, but that's for another PR

orm/lib/Doctrine/ORM/AbstractQuery.php

Lines 709 to 726 in 9f555ea

	/**
	* Set whether or not to cache the results of this query and if so, for
	* how long and which ID to use for the cache entry.
	*
	* @deprecated 2.7 Use {@see enableResultCache} and {@see disableResultCache} instead.
	*
	* @param bool $useCache Whether or not to cache the results of this query.
	* @param int $lifetime How long the cache entry is valid, in seconds.
	* @param string $resultCacheId ID to use for the cache entry.
	*
	* @return $this
	*/
	public function useResultCache($useCache, $lifetime = null, $resultCacheId = null)
	{
	return $useCache
	? $this->enableResultCache($lifetime, $resultCacheId)
	: $this->disableResultCache();
	}

[derrabus]

Yes, please send a PR. 🙂

[greg0ire]

everytime when we change an internal variable name of an argument when it's a method with just one argument.

How often are we going to do that? Because of named arguments, this is arguably a breaking change now.

[SenseException]

How often are we going to do that? Because of named arguments, this is arguably a breaking change now.

I suppose not that often, but it's still a bad practice to use this when we only have one argument or when all arguments are already covered. Question is if we keep named arguments for context or just use a variable again.

[greg0ire]

Why is it a bad practice to use named arguments when there is only one argument? I mean in some situations, it might make things clearer. It's not always obvious what an argument is. For instance, if you pass 60, it can be handy to know we are talking seconds and not minutes.

[SenseException]

That's why I ask if this should be kept for context. My change request still included the named arguments.

[greg0ire]

Thanks @kaznovac !