On Fri, Mar 11, 2016 at 10:21 AM, Paul Tan <pyoka...@gmail.com> wrote: >> static int config_autostash = -1; > > Hmm, why can't config_autostash just default to 0?
Previously Junio recommended not to explicitly initialize a static to 0 (or NULL). http://thread.gmane.org/gmane.comp.version-control.git/287709/focus=287726 Defaulting config_autostash = 0 will work too as you explained. >> + if (opt_autostash == 1) >> + argv_array_push(&args, "--autostash"); >> + else if (opt_autostash == 0) >> + argv_array_push(&args, "--no-autostash"); > > The precise testing for specific values of -1, 0 and 1 throughout the > code makes me uncomfortable. Ordinarily, I would expect a simple > > argv_array_push(&args, opt_autostash ? "--autostash" : "--no-autostash"); This looks good. I will change accordingly. > Stepping back a bit, the only reason why we introduced opt_autostash = > -1 as a possible value is because we need to detect if > --[no-]autostash is being used with git-merge. I consider that a > kludge, because if git-merge supports --autostash as well (possibly in > the future), then we will not need this -1 value. > > So, from that point of view, a -1 value is okay as a workaround, but > kludges, and hence the -1 value, should be gotten rid off as soon as > possible. That right! But until git-merge doesn't support --autostash, it's necessary to have opt_autostash = -1 as default. I wonder if it will be a good thing to add the following line to the commit message "Changes needed to be introduced: Change opt_autostash = 0 as default as soon as git-merge supports --autostash option, also display no error when "git pull --autostash" is called." Possibly it would be better to add gmane link of your review in the commit message for further clarification. Thanks, Mehul -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
