Hi Junio, Le 2022-10-21 à 17:06, Junio C Hamano a écrit : > "Philippe Blain via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> From: Philippe Blain <levraiphilippeblain@xxxxxxxxx> >> >> Some variables in 'test_commit' have names that are common enough that >> it is very likely that test authors might use them in a test. If they do >> so and use 'test_commit' between setting such a variable and using it, >> the variable value from 'test_commit' will leak back into the test and >> most likely break it. >> >> Prevent that by marking all variables in 'test_commit' as 'local'. This >> allow a subsequent commit to use a 'tag' variable. > > This is the right thing to do, if done onn day 1 of the project, and > it is the right thing to do for the longer term health of the > project. But it is a bit scary thing to do in the middle. > > I wonder if there is an easy way to detect that a set of callers of > test_commit are relying on the fact that calling test_commit without > passing --author option cleared their $author variable (similarly > for other variables that are cleared or set to a known value as a > side effect of calling test_commit). If their correctness depends > on $author becoming empty after calling the script today, they will > get broken by this change. > > While it is OK to argue that they deserve it, we would have to be > finding and fixing them ourselves after all. Isn't the fact that the test suite passes with this change enough for us to be confident that nothing is broken by it ? In any case, I did a quick manual grep of each of the variables and could not find a test that uses these names, so I think we are safe. Thanks, Philippe.
