On Fri, Oct 19, 2018 at 06:33:30PM +0200, Duy Nguyen wrote: > > This function doesn't actually make use of the newly added "istate" (nor > > does it use the_index, so there's not a spot that forgot to get > > converted). Is this in preparation for more refactoring, or is it just a > > mistake and can be dropped? > > It's possible that it was needed at some point when I converted diff > api to pass index_state around. But in later iterations I think I > passed "struct repository *" instead because diff code needed much > more than the index, but did not clean this up. Sorry. > > In this function, we still have the_repository for > repo_init_revisions() and it should be gone eventually. "struct > repository *r" could take the place of "struct index_state *istate" > instead and we would need to reopen the path again. > > So I'm not sure, if it's really bad, we could remove it now. Otherwise > we could just leave it there, I don't think this "istate" will survive > very long. I already started deleting some of the_repository in "kill > the_index" series. OK, that all makes sense. I need to either drop it or mark it unused to satisfy -Wunused-parameters. I think I'd lean toward the former, if it's not likely to be used (even if we add "struct repository" later, the separate index parameter would just go away then). > > I don't see anything in the "Kill the_index, final part" series that > > would need it. > > Yeah. Killing the_index is just the first small step (didn't look that > small when I started). Now it's all about the_repository ;-) and we > have like 400 references of it just in library code. I suspect it is much worse than that, even. There are many spots that likely are relying on global data that _should_ be in the repository struct but aren't yet. I don't think there's even an easy way to count those. ;) -Peff
