Re: [PATCH 4/4] bisect--helper: `bisect_reset` shell function in C

On Wed, Jun 8, 2016 at 9:20 AM, Pranit Bauva [off-list ref] wrote:

On Wed, Jun 8, 2016 at 1:29 PM, Eric Sunshine [off-list ref] wrote:

quoted

On Tue, Jun 7, 2016 at 4:54 PM, Pranit Bauva [off-list ref] wrote:

quoted

+       if (!file_exists(git_path_bisect_head())) {
+               struct argv_array argv = ARGV_ARRAY_INIT;
+               argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL);
+               status = run_command_v_opt(argv.argv, RUN_GIT_CMD);
+               argv_array_clear(&argv);
+       }
+
+       if (status) {

What's the purpose of having this 'status' conditional outside of the
above !file_exists() conditional, which is the only place that
'status' gets assigned. Likewise, 'status' itself could be declared
within the scope of that conditional block.

I wanted to avoid nesting. In a few other parts of the code also,
nesting is avoided as much as possible.

I figured as much, but I'm not sure that that is such a good idea in
this case since it makes the code more difficult to reason about. To
wit: as a reader of the code, I spent extra time trying to figure why
it was structured this way and if 'status' is assigned or accessed in
some other non-obvious way.