From: Jakub Narebski <jnareb@gmail.com>
Date: Sun, 29 Jul 2018 21:07:49 GMT
Subject: Re: [PATCH 4/8] test-repository: properly init repo
Message-ID: <86pnz57ntm.fsf@gmail.com>
In-Reply-To: <88711a3cf4c045365a40504db94c34675f3bdc4e.1531926932.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  t/helper/test-repository.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
> index 2762ca656..6a84a53ef 100644
> --- a/t/helper/test-repository.c
> +++ b/t/helper/test-repository.c
> @@ -15,7 +15,10 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,
>  	struct commit *c;
>  	struct commit_list *parent;
>  
> -	repo_init(&r, gitdir, worktree);
> +	setup_git_env(gitdir);
> +
> +	if (repo_init(&r, gitdir, worktree))
> +		die("Couldn't init repo");

Shouldn't this message be marked for translation?

  +		die(_("Couldn't init repo"));

Perhaps we could provide also more detail about error, if possible.

>  
>  	c = lookup_commit(&r, commit_oid);
>  
> @@ -38,7 +41,10 @@ static void test_get_commit_tree_in_graph(const char *gitdir,
>  	struct commit *c;
>  	struct tree *tree;
>  
> -	repo_init(&r, gitdir, worktree);
> +	setup_git_env(gitdir);
> +
> +	if (repo_init(&r, gitdir, worktree))
> +		die("Couldn't init repo");

I wonder if it would be worth it to extract this idiom into separate
function, called for exaple repo_init_or_die(), to avoid code
duplication (and maybe use it in other places?).

>  
>  	c = lookup_commit(&r, commit_oid);