Re: [PATCH v1 04/27] revision: provide implementation for diff merges tweaks

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Is your naming convention that "rev_diff_" is the common prefix?
> What's the significance of "_diff" part?  After all, these are about
> tweaking the setting in the rev_info structure, so how about
>
> 	revinfo_show_merges_in_dense_combined_by_default(rev);
> 	revinfo_show_merges_by_default_with_first_parent(rev);
>
> perhaps, using just "revinfo_" as the common prefix?

Actually, I'd like to take these suggestions back.  I do not think
these names that describe "what the function does" are good names at
all.  If we can name them after what they are for, we'd be much
better off.

Given that one is part of the tweak for "show" and the other part of
"log", I presume that one is to set-up default for showing a merge
in the context of the "show" command, and the other is the same for
the "log" (actually "log -p") command.  So perhaps

	diff_merges_set_default_for_show(revs);
	diff_merges_set_default_for_log(revs);

are good names that tells us where they should be used and what for.
The former is to set up the default parameters to handle a merge
commit in the context of the "show" command, and the latter is the
same for the "log" command.  Naming them that way, we can then tweak
what should happen for "show" by default without touching the caller
if we wanted to (perhaps, "when doing first-parent traversal, really
pretend that all the commits we see are single-parent, so show the
change relative to its first parent by default without even
requiring -m" might be something the latter can learn later).

Thanks.





[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]

  Powered by Linux