Re: [PATCH 13/16] test-reach: test can_all_from_reach_with_flags

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jul 17, 2018, 00:10 UTC
Message-ID: <20180717001006.8283-1-jonathantanmy@google.com>
In-Reply-To: <95b11ced4c0355aabf458539a284bb3895d73f6f.1531746012.git.gitgitgadget@gmail.com>

The subject should be can_all_from_reach_with_flag (without the "s" at the end). Likewise in the commit message.

> To make this method testable, add a new can_all_from_reach method that
> does the initial setup and final tear-down. Call the method from
> 'test-tool reach'.

This description leads me to believe that can_all_from_reach() is (1) trivial, and (2) will not be used in production code. But (1) the function itself is non-trivial and the function signature contains a "cutoff_by_min_date" parameter not found in can_all_from_reach_with_flag():

> +int can_all_from_reach(struct commit_list *from, struct commit_list *to,
> +		       int cutoff_by_min_date)

and (2) this function will be used in production code subsequently in the "commit-reach: use can_all_from_reach" commit. It would be clearer, maybe, if there were some rearrangement - maybe a commit introducing this function, especially documenting what cutoff_by_min_date does, and then a test (which just tests can_all_from_reach), and then the "commit-reach: use can_all_from_reach" commit.