Hi Taylor, Thanks for taking a look. On Wed, 11 Oct 2023 at 22:32, Taylor Blau <me@xxxxxxxxxxxx> wrote: > > One of the custom patches that GitHub is carrying in its private > fork is something similar to this that dumps information from > upload-pack into a custom logging system specific to GitHub I suspected as much. My initial goal is clones vs fetches, filter use, and shallow use... since I'm changing code is there anything else that you think is worthwhile instrumenting? I thought about casting the ints to booleans (ie: >0 haves; >0 wants; etc) but I figured the actual numbers might come in handy at some point, and changing them back in future would be incompatible. I deliberately didn't add thin/ofs/etc flags: most of that stuff is on by default now so it felt like noise. > > + jw_object_begin(&jw, 0); > > + { > > Is there a reason that we have a separate scope here? I think we may > want to drop this as unnecessary, but it's entirely possible that I'm > missing something here... I just followed the patterns in test-json-writer.c - personally I think since JSON objects/arrays have starts & ends then it's easy to read. But one person's readable is another's coding style violation ;-) > > + jw_object_string(&jw, "filter", list_object_filter_config_name(data->filter_options.choice)); > > I'm pretty sure that list_object_filter_config_name() returns characters > that are safe for JSON-encoding, and/or that jw_object_string() does any > quoting beforehand, but worth checking nonetheless. Yes, jw_object_string() calls append_quoted_string() which deals with escaping and quoting. Thanks, Rob :)