From: SZEDER Gábor Date: Thu, 21 Nov 2019 23:03:26 GMT Subject: Re: [PATCH v3 2/2] commit-graph: use start_delayed_progress() Message-ID: <20191121230326.GW23183@szeder.dev> In-Reply-To: <20191107212614.GC29042@sigill.intra.peff.net> On Thu, Nov 07, 2019 at 04:26:14PM -0500, Jeff King wrote: > On Thu, Nov 07, 2019 at 05:46:58PM +0000, Derrick Stolee via GitGitGadget wrote: > > > From: Derrick Stolee > > > > When writing a commit-graph, we show progress along several commit > > walks. When we use start_delayed_progress(), the progress line will > > only appear if that step takes a decent amount of time. > > > > However, one place was missed: computing generation numbers. This is > > normally a very fast operation as all commits have been parsed in a > > previous step. But, this is showing up for all users no matter how few > > commits are being added. > > This part of the patch is a good thing, and obviously correct. But I > wondered... Agreed. > > The tests that check for the progress output have already been updated > > to use GIT_PROGRESS_DELAY=0 to force the expected output. However, there > > is one test in t6500-gc.sh that uses the test_terminal method. This > > mechanism does not preserve the GIT_PROGRESS_DELAY environment variable, > > Why doesn't GIT_PROGRESS_DELAY make it through? Overall it's not that > big a deal to me if it doesn't, but in this test: But I was wondering this, too. If I run the following test: ( write_script script <<-\EOF && echo "GPD: $GIT_PROGRESS_DELAY" EOF GIT_PROGRESS_DELAY=42 && export GIT_PROGRESS_DELAY && test_terminal ./script dummy-arg ) && then its output looks like this: + write_script script + GIT_PROGRESS_DELAY=42 + export GIT_PROGRESS_DELAY + test_terminal ./script dummy-arg GPD: 42 So test_terminal does preserve GIT_PROGRESS_DELAY.