On Wed, Mar 1, 2017 at 12:41 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
>> Keep repo-related path handling in one place. This will make it easier
>> to add submodule/multiworktree support later.
>>
>> This automatically adds the "if submodule then use the submodule version
>> of git_path" to other call sites too. But it does not mean those
>> operations are sumodule-ready. Not yet.
>
> s/sumodule/submodule/
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>> refs/files-backend.c | 45 +++++++++++++++++++++++++--------------------
>> 1 file changed, 25 insertions(+), 20 deletions(-)
>>
>> diff --git a/refs/files-backend.c b/refs/files-backend.c
>> index 7b4ea4c56..72f4e1746 100644
>> --- a/refs/files-backend.c
>> +++ b/refs/files-backend.c
>> @@ -1180,6 +1180,18 @@ static void files_reflog_path(struct files_ref_store *refs,
>> strbuf_git_path(sb, "logs/%s", refname);
>> }
>>
>> +static void files_refname_path(struct files_ref_store *refs,
>> + struct strbuf *sb,
>> + const char *refname)
>> +{
>> + if (refs->submodule) {
>> + strbuf_git_path_submodule(sb, refs->submodule, "%s", refname);
>> + return;
>> + }
>> +
>> + strbuf_git_path(sb, "%s", refname);
>> +}
>
> Maybe it's just me, but I find it odd to exit early here when the first
> exit isn't due to an error. For me, structuring this like `if ()
> call1(); else call2();` would make it clearer that the two code paths
> are equally-valid alternatives, and either one or the other will be
> executed.
Its original version probably looked better. This is another case of
future patches influencing back the past ones: I structure the patch
so that in future we mostly add lines, or delete whole (in this case,
I believe), not modify a lot of lines. I think the readability does
not degrade too much though, so it's probably ok.
--
Duy