Re: [Outreachy][PATCH] builtin/update-server-info: remove the_repository global variable
- From
Usman Akinyemi <usmanakinyemi202@gmail.com>
- Date
- Feb 10, 2025, 18:03 UTC
- Message-ID
- <CAPSxiM8XOH9ueeYwhdQx6PKUWkRbzZh77ZxAmNjSjXrR0gd9_A@mail.gmail.com>
- In-Reply-To
- <xmqqikphbu6b.fsf@gitster.g>
On Mon, Feb 10, 2025 at 10:42 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
> > Usman Akinyemi <usmanakinyemi202@gmail.com> writes: > > > Remove the_repository global variable in favor of the repository > > argument that gets passed in "builtin/upload-server-info.c". > > update? upload? > > I somehow thought that dumb HTTP walker support was on the chopping > list for Git 3.0 but apparently it isn't, so updating this remote > corner of the system I thought nobody cared about is a good thing.
Luckily, there are some other files that have similar structure. I will be sending patches soon for those also.
Show 29 quoted lines
> > I personally feel that from here ... > > > The RUN_SETUP macro is used in "git.c" when the 'update-server-info' > > command is wired to the 'cmd_update_server_info()' function." > > This means we can be sure that the `run_builtin()` function inside > > "git.c" will always pass a valid `repo` variable to `cmd_update_server_info()` > > when the `update-server-info` command is run inside a Git repository. > > > > When the command is run outside a Git repository without the `-h` > > option, the command will fail (`die`) inside the `run_builtin()` function > > when the `setup_git_directory()` is called. So, the `cmd_update_server_info()` > > would not be called at all. > > ... to here are way too verbose and unnecessary. > > > When `-h` is passed to the command outside a > > Git repository, the `run_builtin()` will call the `cmd_update_server_info()` > > function with `repo` set as NULL. > > "set as NULL" -> "set to NULL"? > > ... and then early in the function, "parse_options()" call will give > the options help and exit, without having to consult much of the > configuration file. So it is safe to omit reading the config > when `repo` argument the caller gave us is NULL. > > and that would be sufficient. All the rest of the proposed commit > log message can also be removed, I think.
Yeah, thanks for the review. I will send the updated version. Thanks.
> > Thanks.