Re: [PATCH v2 2/2] format-patch: add format.cover-letter configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:

> Also, add a new option: 'auto', so if there's more than one patch, the
> cover letter is generated, otherwise it's not.

Very sensible goal.

> This has the slight disadvantage that a piece of code will always be run
> even if the user doesn't want a cover letter, and thus waste a few
> cycles.

I am not sure what overhead you are referring to.

We need to count how many we are emitting with or without the cover
letter, and count is stored in "total".  Even when the user said
"auto" (which I personally think should become the default in the
longer term, but that is a separate issue), we shouldn't have to
spend any extra cost if you moved the code that does anything heavy
for cover letter generation after we know what that "total" is, no?

I think the reason you did not move the "find the branch for use in
the cover letter" code down was because it uses the rev->pending
interface, which you cannot read out of after you count the commits,
but that logic to use rev->pending predates the introduction of a
more modern rev->cmdline mechanism, which is already used by the
find_branch_name() function in the same codepath, and it is not
clobbered by prepare_revision_walk().

So perhaps by moving that code down after we know what value "total"
has, and rewriting "what was the positive commit the user gave us"
logic using rev->cmdline, you do not have to do unnecessary work at
all.



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]