Patrick Steinhardt <ps@xxxxxx> writes: > 1. We want to take control of the reference updates so that we can > atomically update all or a subset of references that git-fetch > would have updated. > > 2. We want to be able to quarantine objects in a fetch so that we > can e.g. perform consistency checks for them before they land in > the main repository. If you want to do this, something that might be possible is to change the RHS of the refspecs to put the refs in a namespace of your choice (e.g. ...:refs/<UUID>/...) and then you can look at what's generated and process them as you wish. > - There should be as few global state as possible. This is to reduce > confusion and having to repeat the same incantations in multiple > different locations. Makes sense. > - The logic should be as self-contained as possible. This is so that > it can easily be changed in a subsequent patch series. Also makes sense, but I think that some of your patches might be contrary to this goal (more details below). I've read all the patches, but will just summarize my thoughts here. > Patrick Steinhardt (8): > fetch: rename `display` buffer to avoid name conflict One other way, as others have discussed, is to just name the new variable display_state. (I would prefer that, at the very least so that in case someone else has a patch that contains the identifier "display", problems would be more easily noticed. This is very unlikely to happen but I think it's a good general direction for the Git project to follow.) > fetch: move reference width calculation into `display_state` > fetch: move output format into `display_state` > fetch: pass the full local reference name to `format_display` All these are good changes that I would be happy to see merged. > fetch: deduplicate handling of per-reference format I'm not so sure that this is the correct abstraction. I think that this and the last patch might be counterproductive to your stated goal of having one more mode of printing the refs, in fact, since when we have that new mode, the format would be different but the printing would remain (so we should split the format and printing). > fetch: deduplicate logic to print remote URL Makes sense, although I would need to consider only storing the raw URL in the struct display_state and processing it when it needs to be emitted (haven't checked if this is feasible, though). > fetch: fix inconsistent summary width for pruned and updated refs This changes the behavior in that the summary width, even when printing the summary of pruned refs, is computed based only on the updated refs. The summary width might need to remain out of the struct display_state for now. > fetch: centralize printing of reference updates Same as "fetch: deduplicate handling of per-reference format".
