From: K Jayatheerth Date: Sat, 20 Jun 2026 16:30:09 GMT Subject: Re: [GSoC Patch v6 1/4] path: introduce append_formatted_path() for shared path formatting Message-ID: In-Reply-To: Hi Junio, > It often, even though not always, is a sign of a bad topic structure > to have an insertion-only patch without any removal of existing > code, that adds totally unused code. > > If the step is to "extract the core algorithm", shouldn't it be able > to replace existing code already? Giving the helper and converting its first caller (`rev-parse`) in the same step proves the implementation avoids leaving unused code lingering in the tree, even temporarily. > We may want to add new features to this helper function near the end > of the topic, but wouldn't it make sense for the topic to first > consolidate various path formatting logic already present in the > existing code into a single helper for ease of extending it (which > means replacing open-coded logic in existing code paths with a call > to the new helper, which would have a code that may look very > similar to the original code that was replaced with a single call to > the helper function), and then expose the helper for use by new > callers, and finally further add new features that existing code > paths wouldn't have needed but the new callers would want? Consolidating the existing logic first ensures we aren't introducing unnecessary complexity up front. I agree with restructuring the topic this way. > How else can we make sure this new implementation added by the first > step in the series is (1) capable enough to reproduce what we > already have in different parts of the system, (2) does not bring in > what the current codebase does not need, and (3) bug-to-bug > compatible with the existing code paths? Introducing the helper and swapping out the `rev-parse` implementation in the same step is the best way to prove bug-to-bug compatibility and demonstrate its immediate utility. For v7, I will squash patches 1 and 2 together so that the extraction and the replacement happen simultaneously, guaranteeing that the new `append_formatted_path()` perfectly mirrors the old behavior before we introduce the new `path.*` callers. Thanks for taking the time to explain the rationale! - K Jayatheerth