Re: [PATCH v5 2/3] receive-pack: move message generation to separate function

Junio C Hamano [off-list ref] writes:

Karthik Nayak [off-list ref] writes:

r>> After git-receive-pack(1) has committed the reference updates, we call

quoted

either `report()` or `report_v2()` to report to the client which of the
references we have updated successfully and which updates have failed.
The only difference between those two functions is that the latter also
knows to provide a more detailed report about how exactly a given
reference was updated.

I am torn between praising "bool detailed_report" and frowning on
it.  As the above describes, the difference in behaviour between
report() and report_v2() is if they emit details of per-command
update status, so in that sense, the word "detail" in the name of
the parameter that controls how much details the shared helper
function gives sounds very much appropriate.  On the other hand, the
difference in purpose in these two functions is which version of the
receive-pack protocol they speak, and "This parameter controls how
much detail the report contains" may tempt careless developers into
adding random new pieces of information and break existing clients.
It may be more honest to give it a name that hints that it is about
the protocol version.

Using

    enum report_version {
	receive_pack_report_v0,
	receive_pack_report_v2,
    };

might allow future extension, but it may be overkill.  I dunno.

I think your reasoning does make sense and while the code is _okay_ and
this is probably an overkill like you mentioned, it does draw the
differentiation being a protocol level change rather than a mere boolean
which either protocol could use. Let me add it in if I re-roll.