On Tue, Jan 09, 2024 at 04:29:57PM +0100, Rubén Justo wrote: > Soon we're going to need to pass configuration values to a command in > test-tool. > > Let's teach test-tool to take config values via command line arguments. I wasn't expecting a step like this to appear in this series. I don't have strong feelings about it, especially since test-tool helpers already understand $GIT_DIR/config when they rely on library code which implicitly reads configuration. But it does seem odd to have test-tool invocations that intimately depend on a particular set of configuration values. At the very least, this step seems to encourage passing finely tuned configuration values to test-tool helpers, which I am not sure is a good idea. Your patch message suggests that this will be useful in the following patch, which makes sense. But I wonder if it would be easier to avoid the test-tool entirely and call some Git command in a state that we expect to generate advice. Then we can test its output with various values of advice.adviceOff. > Signed-off-by: Rubén Justo <rjusto@xxxxxxxxx> > --- > t/helper/test-tool.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) The patch itself looks reasonable, though. Thanks, Taylor
