On Mon, Nov 21, 2016 at 4:16 PM, Brandon Williams <bmw...@google.com> wrote: > On 11/21, Stefan Beller wrote: >> On Mon, Nov 21, 2016 at 4:11 PM, Brandon Williams <bmw...@google.com> wrote: >> > On 11/21, Stefan Beller wrote: >> >> >> >> switch (lookup_type) { >> >> @@ -448,7 +448,8 @@ static const struct submodule *config_from(struct >> >> submodule_cache *cache, >> >> >> >> /* fill the submodule config into the cache */ >> >> parameter.cache = cache; >> >> - parameter.commit_sha1 = commit_sha1; >> >> + // todo: get the actual tree here: >> > >> > s/todo/TODO >> > >> > Makes it more clear that this is a TODO >> > >> >> The // is more annoying here. I'll review these changes closely >> before sending out v3. > > Well I prefer // to /* */ but that's not the style we use :)
background: Initially I assume we would need to do work here as that part is exposed to the user in error messages, such that we maybe want to go the reverse direction and not state a tree but instead the commit containing it. Since then I decided that it is not worth to optimize for some hypothetical future and hence I did not go with the internal todo note I put there. Then I forgot about it and it just showed up in the patch here. Having looked through the patch again, the rest looks clean to me. Stefan > > -- > Brandon Williams