Add --json option to fetch
#5974
Conversation
chrisd8088
left a comment
chrisd8088
left a comment
There was a problem hiding this comment.
This looks awesome. Thank you so much!
I've left a few comments and suggestions, but nothing major, I think. If it's easier for you, I can push a commit or two with those minor revisions and then merge the PR; just let me know what you prefer.
I should also say, thanks very much for straightening up all the --json and -j options and their documentation!
| if d.transfers != nil { | ||
| d.transfers = append(d.transfers, t) | ||
| } | ||
| if d.virtuallyFetched != nil { | ||
| d.virtuallyFetched[t.Oid] = true | ||
| } | ||
| if fetchDryRunArg { |
There was a problem hiding this comment.
I hemmed and hawed a bit over this, because append() will accept a nil slice and just allocate a backing array, so we could drop the make([]*tq.Transfer, 0) in the newFetchWatcher() function above, and then do something like:
if fetchJsonArg {
d.transfers = append(d.transfers, t)
}
if fetchDryRunArg {
d.virtuallyFetched[t.Oid] = true
printProgress("%s %s => %s", tr.Tr.Get("fetch"), t.Oid, t.Name)
}However, since in your next PR #5975 you're adding another flag whose state affects both functions, I think it's probably more elegant to leave it as you've written it.
There was a problem hiding this comment.
Fair point, I'll mull it over when I get back to that PR, maybe I can indeed move choosing the behaviour from the constructor to this function. For now, I left it like it was.
2 participants
This adds a
--jsonoption to fetch, which is done on top of #5973.It turned out there was a minor bug in
--dry-run, where the same object was being reported fetched multiple times from different references, which diverged from non-dry-run behaviour, where the object being fetched for one ref prevented refetching for the other. This has been fixed by keeping a set of virtually fetched OIDs. This and gathering the transfers across references required sharing afetchWatcherobject acrossfetch*calls.Additionally, this PR also harmonizes the
--jsonflag occurring in different commands:-jas a shorcut (previously onlystatusdid)