Re: cmd_cherry in builtin/log.c?

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Dec 9, 2010, 21:14 UTC
Message-ID: <4D0146BA.2090801@lsrfire.ath.cx>
In-Reply-To: <7vaakhpbq2.fsf@alter.siamese.dyndns.org>

Am 07.12.2010 21:30, schrieb Junio C Hamano:

Show 19 quoted lines

> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Am 07.12.2010 18:39, schrieb Jonathan Nieder:
>>> while the core of the builtin version is
>>>
>>> 	get_patch_ids(&revs, &patch_id_opts, prefix);
>>>
>>> The latter function is static, introduced by v1.4.1~12^2~5
>>> (format-patch: introduce "--ignore-if-in-upstream", 2006-06-25).
>>>
>>> So the answer is that "git cherry" is considered a variant on
>>> "git log" (like format-patch, show, and whatchanged) and that it uses
>>> "git log" internals.
>>
>> That's right.  get_patch_ids() could be moved into patch-ids.c now and
>> then cmd_cherry() could get its own file in builtin/, though.
> 
> Right, but the key word is "could".  Is it hurting _anything_ to have it
> in the current place?  I doubt it.

Indeed. Moving cherry's code into its own file is be a code clean up. There would be no benefit to users, future developers should have a somewhat easier time navigating the code while any developers currently working on the code would get it pulled from under them.

Such a clean up can be useful to include at the start of a patch series that contains actual user visible improvements.

René