Re: [PATCH] Make pull.c match the structural conventions

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 12, 2025, 04:50 UTC
Message-ID: <xmqqikeccnhx.fsf@gitster.g>
In-Reply-To: <20251212020930.11654-1-jayatheerthkulkarni2005@gmail.com>

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

Show 15 quoted lines

> The builtin sources follow a predictable structure, and pull.c departs
> from that pattern by arranging its option table in a way that disrupts
> the expected flow of the file. The irregular placement makes the file
> harder to read, breaks the visual rhythm shared by other builtins, and
> forces readers to jump around to understand how options are handled.
> The lack of consistency makes pull.c feel like an outlier rather than
> a peer alongside the other commands.
>
> A consistent layout helps readers rely on established mental models,
> so bringing pull.c into alignment improves clarity and makes the file
> easier to navigate and maintain.
>
> Pull.c, become structured like the other builtin/*.c files, keeping the
> option definitions where the reader naturally expects them and restoring
> the uniformity of the builtin command layout.

The above is, what should we say, overhyped? I do not know an appropriate phrase, but there are subjective judgements without backing it up with exactly which pattern the code "departs from".

In other words, too many adjectives, so little substance.

I expected something a lot more than a simple change that can be summarized a lot more concisely, like

    Unless there are good reasons, it is customary to have the
    options[] array given to parseopt API in the function scope,
    not in the file scope.

    Make builtin/pull.c:cmd_pull() to follow that convention.

or something.