Hi Stephan, On Wed, Dec 7, 2016 at 12:40 AM, Stephan Beyer <s-beyer@xxxxxxx> wrote: > Hi Pranit and Christian and Lars ;) > > On 12/07/2016 12:02 AM, Pranit Bauva wrote: >> On Tue, Nov 22, 2016 at 6:19 AM, Stephan Beyer <s-beyer@xxxxxxx> wrote: >>> >>> Now that I have a coarse overview of what you did, I have the general >>> remark that imho the "terms" variable should simply be global instead of >>> being passed around all the time. >> >> In a personal conversation with my mentors, we thought it is the best >> fit to keep it in a struct and pass it around so that it is easier in >> libification. > > I guess you had that conversation at the beginning of the project and I > think that at that stage I would have recommended it that way, too. > > However, looking at the v15 bisect--helper.c, it looks like it is rather > a pain to do it that way. I mean, you use the terms like they were > global variables: you initialize them in cmd_bisect__helper() and then > you always pass them around; you update them "globally", never using > restricted scope, never ever use a second instantiation besides the one > of cmd_bisect__helper(). These are all signs that *in the current code* > using (static) global variables is the better way (making the code simpler). > > The libification argument may be worth considering, though. I am not > sure if there is really a use-case where you need two different > instantiations of the terms, *except* if the lib allows bisecting with > different terms at the same time (in different repos, of course). Is the > lib "aware" of such use-cases like doing stuff in different repos at the > same time? If that's the case, not using global variables is the right > thing to do. In any other case I can imagine, I'd suggest to use global > variables. Yeah, I wanted to avoid global variables when there is no very good reason for them to avoid possible libification work later. And I don't think passing one parameter around to many functions is a big burden. Thanks for your reviews, Christian.