Re: [PATCH] name-rev: use generation numbers if available

From: Junio C Hamano <gitster@pobox.com>
To: "Keller, Jacob E" <jacob.e.keller@intel.com>
Cc: Derrick Stolee <derrickstolee@github.com>,
	Jacob Keller <jacob.keller@gmail.com>,
	Git mailing list <git@vger.kernel.org>
Subject: Re: [PATCH] name-rev: use generation numbers if available
Date: Wed, 02 Mar 2022 17:10:37 -0800	[thread overview]
Message-ID: <xmqqilsvet82.fsf@gitster.g> (raw)
In-Reply-To: <CO1PR11MB5089DC997DB42023324F1BF0D6029@CO1PR11MB5089.namprd11.prod.outlook.com> (Jacob E. Keller's message of "Tue, 1 Mar 2022 22:46:06 +0000")

"Keller, Jacob E" <jacob.e.keller@intel.com> writes:

> Let's clarify. There are two versions of the test in this version:
>
> 1) test which enables commit graph and tests that it does the right behavior.
>
> 2) test which removes commit graph and tests that it behaves the old way.
>
>
> test (1) checks the flow with the commit graph enabled, and verifies that with a commit graph the new behavior is used. This test will fail if you revert the name-rev commit-graph support.
>
> test (2) always performs the way we don't like. (since we disable the commit graph and the new flow doesn't kick in) This is the test I think I will eliminate in the next revision.
>
>
> I will remove the 2nd test, since the first test covers the change
> in behavior just fine, and I think I agree we don't need to set
> in-stone the implementation without commit graph.
>
> I will also look at adding a test which performs a count of which
> revisions get inspected and makes sure that we actually are doing
> the optimization.

Sounds like a sensible thing to do.

In any case, in the current patch, #2 is not working in
linux-TEST-vars job at CI.  You can visit this URL

https://github.com/git/git/runs/5400048732?check_suite_focus=true#step:4:68062

while logged into your GitHub account for details.

Thanks.