On 08/01, Junio C Hamano wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> 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.
>
>
> >>> 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
