Hi Liam,
On Tue, 28 Nov 2017, liam Beguin wrote:
> On 27/11/17 06:04 PM, Johannes Schindelin wrote:
> >
> > On Sun, 26 Nov 2017, Liam Beguin wrote:
> >
> >> @@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out,
> >> strbuf_reset(&buf);
> >> if (!keep_empty && is_original_commit_empty(commit))
> >> strbuf_addf(&buf, "%c ", comment_line_char);
> >> - strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid));
> >> + strbuf_addf(&buf, "%s %s ",
> >> + abbreviate_commands ? "p" : "pick",
> >> + oid_to_hex(&commit->object.oid));
> >
> > I guess the compiler will optimize this code so that the conditional
> > is evaluated only once. Not that this is performance critical ;-)
>
> Is your guess enough? :-) If not, how could I make sure this is
> optimized? Should I do that check before the while() loop?
I am a fan of not relying too heavily on compiler optimization and e.g.
extract code from loops when it does not need to be evaluated every single
iteration. In this case:
const char *pick = abbreviate_commands ? "p" : "pick";
...
strbuf_addf(&buf, "%s %s ", pick,
oid_to_hex(&commit->object.oid));
But given Junio's comment that the assignment of `first` was too far away
from the line where it is used for his taste, I guess he will argue (once
again) the exact opposite of me.
Ciao,
Dscho
