* cmd_cherry in builtin/log.c?
@ 2010-12-07 16:02 Thiago Farina
2010-12-07 17:39 ` Jonathan Nieder
0 siblings, 1 reply; 7+ messages in thread
From: Thiago Farina @ 2010-12-07 16:02 UTC (permalink / raw)
To: Git Mailing List; +Cc: rene.scharfe
I was looking into builtin/log.c to see how it does --reverse and I
saw that cmd_cherry is there.
I'm wondering, why is it there? What was the reason to make the
decision to put it there and not in builtin/cherry.c?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cmd_cherry in builtin/log.c?
2010-12-07 16:02 cmd_cherry in builtin/log.c? Thiago Farina
@ 2010-12-07 17:39 ` Jonathan Nieder
2010-12-07 18:57 ` René Scharfe
2010-12-08 11:57 ` Thiago Farina
0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Nieder @ 2010-12-07 17:39 UTC (permalink / raw)
To: Thiago Farina; +Cc: Git Mailing List, rene.scharfe
Hi,
Thiago Farina wrote:
> I was looking into builtin/log.c to see how it does --reverse and I
> saw that cmd_cherry is there.
>
> I'm wondering, why is it there?
Good question. So let's check.
$ git log --oneline -Scmd_cherry builtin/log.c
81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory
That wasn't too helpful. Okay, okay.
$ git log --oneline -Scmd_cherry -- builtin-log.c
81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory
e827633 Built-in cherry
Running "git show e827633" reveals that the core of the original
script is
for c in $inup
do
git-diff-tree -p $c
done | git-patch-id |
while read id name
do
echo $name >>$patch/$id
done
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.
Hope that helps,
Jonathan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cmd_cherry in builtin/log.c?
2010-12-07 17:39 ` Jonathan Nieder
@ 2010-12-07 18:57 ` René Scharfe
2010-12-07 20:30 ` Junio C Hamano
2010-12-08 11:57 ` Thiago Farina
1 sibling, 1 reply; 7+ messages in thread
From: René Scharfe @ 2010-12-07 18:57 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Thiago Farina, Git Mailing List
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.
René
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cmd_cherry in builtin/log.c?
2010-12-07 18:57 ` René Scharfe
@ 2010-12-07 20:30 ` Junio C Hamano
2010-12-09 21:14 ` René Scharfe
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2010-12-07 20:30 UTC (permalink / raw)
To: René Scharfe; +Cc: Jonathan Nieder, Thiago Farina, Git Mailing List
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cmd_cherry in builtin/log.c?
2010-12-07 17:39 ` Jonathan Nieder
2010-12-07 18:57 ` René Scharfe
@ 2010-12-08 11:57 ` Thiago Farina
2010-12-08 18:09 ` Jonathan Nieder
1 sibling, 1 reply; 7+ messages in thread
From: Thiago Farina @ 2010-12-08 11:57 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git Mailing List, rene.scharfe
On Tue, Dec 7, 2010 at 3:39 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Thiago Farina wrote:
>
>> I was looking into builtin/log.c to see how it does --reverse and I
>> saw that cmd_cherry is there.
>>
>> I'm wondering, why is it there?
>
> Good question. So let's check.
>
> $ git log --oneline -Scmd_cherry builtin/log.c
> 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory
>
> That wasn't too helpful. Okay, okay.
>
> $ git log --oneline -Scmd_cherry -- builtin-log.c
> 81b50f3 Move 'builtin-*' into a 'builtin/' subdirectory
> e827633 Built-in cherry
>
> Running "git show e827633" reveals that the core of the original
> script is
>
> for c in $inup
> do
> git-diff-tree -p $c
> done | git-patch-id |
> while read id name
> do
> echo $name >>$patch/$id
> done
>
> 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.
>
Yup, thanks for digging into it. Now makes sense.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cmd_cherry in builtin/log.c?
2010-12-08 11:57 ` Thiago Farina
@ 2010-12-08 18:09 ` Jonathan Nieder
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Nieder @ 2010-12-08 18:09 UTC (permalink / raw)
To: Thiago Farina; +Cc: Git Mailing List, rene.scharfe
Thiago Farina wrote:
> Yup, thanks for digging into it. Now makes sense.
Mm, I was hoping for "thanks for teaching me to dig into it". :)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: cmd_cherry in builtin/log.c?
2010-12-07 20:30 ` Junio C Hamano
@ 2010-12-09 21:14 ` René Scharfe
0 siblings, 0 replies; 7+ messages in thread
From: René Scharfe @ 2010-12-09 21:14 UTC (permalink / raw)
Cc: Junio C Hamano, Jonathan Nieder, Thiago Farina, Git Mailing List
Am 07.12.2010 21:30, schrieb Junio C Hamano:
> 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é
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-12-09 21:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-07 16:02 cmd_cherry in builtin/log.c? Thiago Farina
2010-12-07 17:39 ` Jonathan Nieder
2010-12-07 18:57 ` René Scharfe
2010-12-07 20:30 ` Junio C Hamano
2010-12-09 21:14 ` René Scharfe
2010-12-08 11:57 ` Thiago Farina
2010-12-08 18:09 ` Jonathan Nieder
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.