From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Wed, 15 Feb 2017 02:06:47 GMT
Subject: Re: [PATCH 09/14] update submodules: add submodule_go_from_to
Message-ID: <20170215020647.dn2drkep7o7qwqp7@genre.crustytoothpaste.net>
In-Reply-To: <20170215003423.20245-10-sbeller@google.com>

On Tue, Feb 14, 2017 at 04:34:18PM -0800, Stefan Beller wrote:
> +	prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> +
> +	cp.git_cmd = 1;
> +	cp.no_stdin = 1;
> +	cp.dir = path;
> +
> +	argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
> +	argv_array_pushl(&cp.args, "read-tree", NULL);
> +
> +	if (!dry_run)
> +		argv_array_push(&cp.args, "-u");
> +	else
> +		argv_array_push(&cp.args, "-n");

I might write this as

	if (dry_run)
		argv_array_push(&cp.args, "-n");
	else
		argv_array_push(&cp.args, "-u");

In other words, avoiding the negation when you have an else branch.  I
can also see an argument for keeping the condition identical to the
other branches, though.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204