Re: [PATCH] convert any hard coded .gitmodules file string to the MACRO

From: Brandon Williams <bmwill@google.com>
Date: Aug 2, 2017, 17:46 UTC
Message-ID: <20170802174617.GC36159@google.com>
In-Reply-To: <xmqq4ltrf30q.fsf@gitster.mtv.corp.google.com>

On 08/01, Junio C Hamano wrote:

Show 18 quoted lines

> Stefan Beller <sbeller@google.com> writes:
> 
> >>>   @@ -233,18 +233,18 @@ void gitmodules_config(void)
> >>>                 strbuf_addstr(&gitmodules_path, "/.gitmodules");
> >>
> >>
> >> Did you mean to also change "/.gitmodules" ??
> >
> > Goog point. We should pick that up as well. However as we do not have
> > a macro for that, we'd have to have 2 calls to strbuf API
> >
> >     strbuf_addch(&sb, '/');
> >     strbuf_addstr(&sb, GITMODULES);
> 
> Ehh, doesn't string literal concatenation work here?  I.e. something
> like:
> 
>     strbuf_addstr(&gitmodules_path, "/" GITMODULES_FILE);

Also this doesn't really matter much since this line is removed latter on in the series, but I'll go with the string literal concatenation for the intermediate state.

Show 20 quoted lines

> 
> 
> >>>                 if (pos < 0) { /* .gitmodules not found or isn't merged */
> >>>                         pos = -1 - pos;
> >>>                         if (active_nr > pos) {  /* there is a .gitmodules
> >>> */
> >>
> >>
> >> It might also be nice to change the literals in the comments to
> >> use the macro.
> 
> The reason you want this patch is not like we want to make it easy
> to rename the file to ".gitprojects" later, right?  The patch is
> about avoiding misspelled string constant, like "/.gitmdoules",
> without getting caught by the compiler, no?
> 
> Assuming that I am correctly guessing the intention, I think it is a
> bad idea to rename these in the comments.
> 
>

-- 
Brandon Williams