On 02/20, Stefan Beller wrote: > 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. I think Jonathan is trying to point out that converting to f(repo) maybe easier than converting to f(repo->object_store) upfront which would make it easier to write the patches (which most of them are already written) and to review because you can use the #define trick to make some sort of guarantees. After we have successfully completed the migration to f(repo), then we can revisit the subsystems which want to have a clearer abstraction layer and make the jump to f(repo->object_store). -- Brandon Williams
