Subject: Re: [PATCH 4/8] test-repository: properly init repo

From: Jakub Narebski <[email-masked]>
Date: Sun, 29 Jul 2018 23:07:49 +0200

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> From: Derrick Stolee <[email-masked]>
>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  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);