Re: [PATCH 12/15] sequencer: lib'ify save_opts()

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2016, 17:44 UTC
Message-ID: <xmqqwpj3h1bg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <7768b55e65c771a5615a6f1209b40395dc705425.1471968378.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 10 quoted lines

>  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;