diff mbox series

[v3,3/3] archive: remove the_repository global variable

Message ID 5d33a375f41132b8b378885d00e934b9f20a0854.1728099043.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Remove the_repository global for am, annotate, apply, archive builtins | expand

Commit Message

John Cai Oct. 5, 2024, 3:30 a.m. UTC 
From: John Cai <johncai86@gmail.com>

Replace the_repository with the repository argument that gets passed
down through the builtin function.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 builtin/archive.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

shejialuo Oct. 5, 2024, 7:13 a.m. UTC | #1 
On Sat, Oct 05, 2024 at 03:30:43AM +0000, John Cai via GitGitGadget wrote:
> diff --git a/builtin/archive.c b/builtin/archive.c
> index dc926d1a3df..13ea7308c8b 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c

[snip]

>  
> -	ret = write_archive(argc, argv, prefix, the_repository, output, 0);
> +	ret = write_archive(argc, argv, prefix, repo, output, 0);
>  

When I read this new series, I feel quite strange for why we only change
"the_repository" to "repo". After reading the comments from [PATCH v2 4/4],
I have understood the context.

I think we should improve the commit message to take about we decide to
remove the "the_repository" from "archive.c" code unless it will bring a
lot of confusion for the reader.

Thanks,
Jialuo
John Cai Oct. 10, 2024, 6:27 p.m. UTC | #2 
Hi Jialuo,

On 5 Oct 2024, at 3:13, shejialuo wrote:

> On Sat, Oct 05, 2024 at 03:30:43AM +0000, John Cai via GitGitGadget wrote:
>> diff --git a/builtin/archive.c b/builtin/archive.c
>> index dc926d1a3df..13ea7308c8b 100644
>> --- a/builtin/archive.c
>> +++ b/builtin/archive.c
>
> [snip]
>
>>
>> -	ret = write_archive(argc, argv, prefix, the_repository, output, 0);
>> +	ret = write_archive(argc, argv, prefix, repo, output, 0);
>>
>
> When I read this new series, I feel quite strange for why we only change
> "the_repository" to "repo". After reading the comments from [PATCH v2 4/4],
> I have understood the context.
>
> I think we should improve the commit message to take about we decide to
> remove the "the_repository" from "archive.c" code unless it will bring a
> lot of confusion for the reader.

Sounds good. I will add more explanation for why we are making the change.

>
> Thanks,
> Jialuo

Thanks
John
diff mbox series

Patch

diff --git a/builtin/archive.c b/builtin/archive.c
index dc926d1a3df..13ea7308c8b 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -2,7 +2,6 @@ 
  * Copyright (c) 2006 Franck Bui-Huu
  * Copyright (c) 2006 Rene Scharfe
  */
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "archive.h"
 #include "gettext.h"
@@ -79,7 +78,7 @@  static int run_remote_archiver(int argc, const char **argv,
 int cmd_archive(int argc,
 		const char **argv,
 		const char *prefix,
-		struct repository *repo UNUSED)
+		struct repository *repo)
 {
 	const char *exec = "git-upload-archive";
 	char *output = NULL;
@@ -110,7 +109,7 @@  int cmd_archive(int argc,
 
 	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
 
-	ret = write_archive(argc, argv, prefix, the_repository, output, 0);
+	ret = write_archive(argc, argv, prefix, repo, output, 0);
 
 out:
 	free(output);