Jonathan Nieder <jrnieder@xxxxxxxxx> writes: >> I was hoping to hear quick Acks for remove-ignore-env (and also >> Duy's reroll of their topics on it) from people involved in all the >> related topics, so that we can advance them more-or-less at the same >> time. > > I can go along with this for this series, but I want to explain why I > am not thrilled with the process in general. > > The series "Moving global state into the repository object (part 1)" > had gone through three revisions with relatively minor changes between > each and we were in the middle of reviewing the fourth. In response > to a series that comes after it, "(part 2)", Duy created > nd/remove-ignore-env, a series that I quite like. > > But with this reroll, the result is: > ... > So even though I agree with the goal and I like the improved > initialization code, I am not happy. Well, I do not think it was the "process" issue in particular, though. Yes, it was unfortunate that these three revisions were reviewed but they each resulted with only relative minor changes between iterations. IOW, reviewers that lost fresh eyes, perhaps including me, all missed a larger wart in the design that made the "ignore-env" thing required in the structure, which was spotted by somebody else. It would have been nicer if such an improvement were done during an earlier part of the cycle and did not require that other reviewer. But we didn't spot it, and now we know a better structure of the topic, luckily before all the things touch 'next'. We not just got a suggestion "it would be better to rewrite it like so---now go ahead to do that", we got something even better and Duy did all the work restructuring the series, reshuffling the patches. We should be thankful, not unhappy.
