From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 29 Aug 2016 12:09:33 GMT
Subject: Re: [PATCH 12/15] sequencer: lib'ify save_opts()
Message-ID: <alpine.DEB.2.20.1608291409210.129229@virtualbox>
In-Reply-To: <xmqqwpj3h1bg.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 26 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> >  static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
> > @@ -1128,9 +1130,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
> >  		return -1;
> >  	if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
> >  		return error(_("Can't revert as initial commit"));
> > -	if (save_head(sha1_to_hex(sha1)))
> > +	if (save_head(sha1_to_hex(sha1)) ||
> > +			save_opts(opts))
> >  		return -1;
> > -	save_opts(opts);
> 
> I think it is much easier to read to keep this on a single line.  It
> would be more verbose but an even easier would be to keep these two
> operations two separate steps, i.e.
> 
>         if (save_head())
>                 return -1;
>         if (save_opts())
>                 return -1;

Done,
Dscho