Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values

On 6/28/26 16:40, Junio C Hamano wrote:

Tian Yuchen [off-list ref] writes:

quoted

quoted

Wouldn't we rather want to try to be more strict and say

	if (!repo || !repo->initialized)
		BUG("repo must be an initialied repository");

here?  Aren't all the callers of this function supposed to be
dealing with an already initialized repository?

That makes sense, but from my point of view...

'repo_config_values()' already has a check for 'repo->initialized'. If
we're absolutely certain that the 'repo' is initialized, wouldn't it be
better to simply remove all the checks inside the getter and leave the
judgment to 'repo_config_values()'?

Yes, that was what I was getting at ;-).

A lot of CI tests are failing, but that just goes to show that the 
"bugs" are being properly identified, doesn’t it?

It means there are a lot of "invalid" calls in the tests (if the way we 
define a 'valid' call, i.e. repo must be initialized, is correct)... It 
seems that code like 'if (repo != the_repository) return' or something 
similar is inevitably going to end up somewhere, even though, as you 
said, it’s "sweeping problems under the rug."

I’m not sure how to proceed from here either..

Regards, yuchen