Edmundo Carmona Antoranz <eantor...@gmail.com> writes:

> Will also affect annotate

Is that a good thing?  In any case, make it understandable without
the title line (i.e. make it a full sentence, ending with a full
stop).

> +                             if (progress) {
> +                                     for (next = suspect->suspects; next != 
> NULL;
> +                                          next = next->next)
> +                                             blamed_lines += next->num_lines;
> +                                     display_progress(progress, 
> blamed_lines);
> +                             }

Is this math and the placement of the code correct?  It would
probably be more obvious if this hunk is in found_guilty_entry(),
which is already the dedicated function in which we report about a
group of lines whose ultimate origin has become clear.

> @@ -2830,11 +2851,11 @@ parse_done:
>  
>       read_mailmap(&mailmap, NULL);
>  
> +     assign_blame(&sb, opt);
> +
>       if (!incremental)
>               setup_pager();
>  
> -     assign_blame(&sb, opt);
> -
>       free(final_commit_name);
>  
>       if (incremental)

Two comments.

 * How does this interact with incremental or porcelain blame?
   Shouldn't progress be turned off when these modes are in use?

 * Shouldn't progress be turned off if the result comes very
   quickly, using start_progress_delay()?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to