Zev Weiss [off-list ref] writes:
Subject: Re: [PATCH 2/5] log: Refactor duplicated code to headerize recipient lists
Style: "log: Refactor" -> "log: refactor"
cf. Documentation/SubmittingPatches[[summary-section]]
The To and Cc headers are handled identically (the only difference is
the header name tag), so we might as well reuse the same code for both
instead of duplicating it.
Makes tons of sense. But seeing this one ...
+ recipients_to_header_buf("To", &buf, &extra_to);
+ recipients_to_header_buf("Cc", &buf, &extra_cc);
... the parameters to the function probably should be ...
+void recipients_to_header_buf(const char *hdr, struct strbuf *buf,
+ const struct string_list *recipients);
... in this order, instead:
format_recipients(&buf, "To", &extra_to);
That is, "To" and &extra_to are much closely related to each other
than they are to &buf in the sense that they are both input to the
helper function to work on, while &buf is an output buffer, and we
tend to place closer things together, next to each other.
Other than that, removal of repetition is very good.
Thanks.