On Mon, Aug 27, 2018 at 8:37 PM Stefan Beller <sbel...@google.com> wrote:
>
> On Sun, Aug 26, 2018 at 3:03 AM Nguyễn Thái Ngọc Duy <pclo...@gmail.com> 
> wrote:
> >
> > This function calls do_diff_cache() which eventually needs to set this
> > "istate" to unpack_options->src_index (*). This is an unfornate fact
>
> unfortunate
>
> > diff --git a/diff.c b/diff.c
>
> Unlike I thought in the cover letter, this is just adding the repository all
> over the place and not adding new code, despite the size.
>
> A cursory read seems to make sense, though I'd nitpick on the
> choice of the variable name as I used to use 'r' for the repository
> struct. I am not saying that is better, but we could think if we want to
> strive for some consistency eventually. (for example most strbufs are
> named 'sb', as they are temporary helpers with no explicit naming
> required. So maybe we could strive to name all "repository pass throughs"
> to be "repo" or "r").

"r" it is! I forgot about it. But this is for local variable or
argument names only right? The field name (in diff_options for
example) should stay something more descriptive like repo, I think.
-- 
Duy

Reply via email to