On 8/3/2022 9:45 PM, Victoria Dye via GitGitGadget wrote: > From: Victoria Dye <vdye@xxxxxxxxxx> > > Create a 'git diagnose' builtin to generate a standalone zip archive of > repository diagnostics. > + * The contents of the `.git`, `.git/hooks`, `.git/info`, `.git/logs`, and > + `.git/objects/info` directories You remove these lines in the next patch, which is called "gate certain data behind '--all'" but maybe we shouldn't have this functionality now and instead add it in the future. The biggest reason for the --all option is that these contents will likely include private IP (path names and branch names, but not file contents) that the user would probably not want to share with the public mailing list, but might want to share with a trusted Git expert in order to resolve a problem. You mention earlier that The generated archive can then, for example, be shared with the Git mailing list to help debug an issue or serve as a reference for independent debugging. So, if you're sending a v3, then moving this out of this patch and into the next one would be a good way to be sure that this possibly-private data is not mentioned as something to share super publicly. (Of course, this requires making the change to create_diagnostics_archive() in advance of creating the builtin, so maybe this reorganization isn't worth it.) > @@ -0,0 +1,58 @@ > +#include "builtin.h" > +#include "parse-options.h" > +#include "diagnose.h" > + > + nit: double empty line > +++ b/t/t0092-diagnose.sh > @@ -0,0 +1,28 @@ > +#!/bin/sh > + > +test_description='git diagnose' > + > +TEST_PASSES_SANITIZE_LEAK=true > +. ./test-lib.sh > + > +test_expect_success UNZIP 'creates diagnostics zip archive' ' > + test_when_finished rm -rf report && > + > + git diagnose -o report -s test >out && > + > + zip_path=report/git-diagnostics-test.zip && > + grep "Available space" out && > + test_path_is_file "$zip_path" && nit: 'grep' the output immediately after the 'git diagnose' command and keep the zip_path use immediately after its definition. Thanks, -Stolee
