Re: [PATCHv3 2/5] connect: use static list of repo-local env vars

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Feb 24, 2010, 07:33 UTC
Message-ID: <cb7bb73a1002232333o2470b8d5x3dd9bd22dd30764e@mail.gmail.com>
In-Reply-To: <4B84D1BD.2030908@viscovery.net>

Doh. I knew I should have waitied a little more before sending the new series ...

On Wed, Feb 24, 2010 at 8:14 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:

> Giuseppe Bilotta schrieb:
>> -             /* remove these from the environment */
>
> You shouldn't remove this comment.

Right. I should probably rephrase it a little, though.

Show 17 quoted lines

>> -             const char *env[] = {
>> -                     ALTERNATE_DB_ENVIRONMENT,
>> -                     DB_ENVIRONMENT,
>> -                     GIT_DIR_ENVIRONMENT,
>> -                     GIT_WORK_TREE_ENVIRONMENT,
>> -                     GRAFT_ENVIRONMENT,
>> -                     INDEX_ENVIRONMENT,
>> -                     NO_REPLACE_OBJECTS_ENVIRONMENT,
>> -                     NULL
>> -             };
>> -             conn->env = env;
>> +             conn->env = local_repo_env;
>
> IMO, you should squash this patch and the previous one under the subject
> "Move list of repo-local environment variables to a public place", and
> that public place could be environment.c instead of a new file cache.c
> (the name "cache.c" is definitely wrong).

The list is now the right place (v4). The squashing is most definitely a good idea, gives a better idea of the code movement.

-- 
Giuseppe "Oblomov" Bilotta