From: Giuseppe Bilotta Date: Wed, 24 Feb 2010 07:33:15 GMT Subject: Re: [PATCHv3 2/5] connect: use static list of repo-local env vars Message-ID: 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 wrote: > Giuseppe Bilotta schrieb: >> -             /* remove these from the environment */ > > You shouldn't remove this comment. Right. I should probably rephrase it a little, though. >> -             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