Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values
From: Tian Yuchen <hidden>
Date: 2026-06-30 16:21:06
Hi Christian and Junio, On 6/29/26 22:47, Junio C Hamano wrote:
Christian Couder [off-list ref] writes:quoted
I agree that the best end state would be to have no `if (!repo || !repo->initialized)` check, but we shouldn't have to get there right away. I think it's fine to proceed in several steps: 1) `if (!repo || !repo->initialized) return NULL;` allows us to remove the global variable and use getters which will help us in the next step. 2) `if (!repo || !repo->initialized) return BUG("repo must be an initialized repository");` now we want to find and fix callers (including tests) that haven't properly initialized things. 3) No `if (!repo || !repo->initialized)` check, as we are sure that all the callers that didn't properly initialized things have been found and fixed. So I think 1) is fine for now as long as we properly explain in the commit messages and in code comments (maybe using NEEDSWORK comments) that we know there is more work to do on this in the future.I am OK with the progressive improvements, but if "wean us away from global variables" topic stops at step 1 I would have to say that is a failed experiment. Not doing (2) means you made the system bug-to-bug compatible from the old world where these things weren't in repo-config but were still globals, which is code churn for nothing good to show in the end result. We need to get to (2) before declaring victory. But of course, we need to start somewhere. (1) with in-code comments sprinkled to such places that say that we'd need to revisit would be a good place to start. Thanks.
Thank you both for paving a clear way forward.
For the upcoming V5 patch, I will implement:
1. Revert to the shields ('return NULL' for the getter, and bypassing
'repo != repository' for the _clear()).
2. Add 'NEEDSWORK' comments above them documenting that these are
temporary changes.
3. Update the commit message to reflect this.
Once this initial migration safely lands, the next goal will be to
investigate those failing CI tests.
Will send out V5 shortly.
Regards, yuchen