On Fri, Feb 16, 2018 at 2:34 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Hi, > > Stefan Beller wrote: > >> v2: >> * duplicated the 'ignore_env' bit into the object store as well >> * the #define trick no longer works as we do not have a "the_objectstore" global, >> which means there is just one patch per function that is converted. >> As this follows the same structure of the previous series, I am still confident >> there is no hidden dependencies to globals outside the object store in these >> converted functions. >> * rebased on top of current master, resolving the merge conflicts. >> I think I used the list.h APIs right, but please double check. > > For what it's worth, I think I prefer v1. I put some comments on why > on patch 0 of v1 and would be interested in your thoughts on them > (e.g. as a reply to that). I also think that even if we want to > switch to a style that passes around object_store separately from > repository, it is easier to do the migration in two steps: first get > rid of hidden dependencies on the_repository, then do the (simpler) > automatic migration from > > f(the_repository) > > to > > f(the_repository->object_store) > > *afterwards*. > > Thoughts? I would prefer to not spend more time on these conversions. If Duy and you would come to a conclusion to either pick this or the previous version I would be happy. I do not see the benefit in splitting up the series even further and do this multistep f(repo) -> f(object store), as the cost in potential merge conflicts is too high. Note that brian just sent another object id conversion series, also touching sha1_file.c, which I am sure will produce merge conflicts for Junio. For the next part 2 and onwards I'd be happy to take either this strategy or Duys strategy as requested. Stefan
