From: Junio C Hamano <gitster@pobox•com>
To: K Jayatheerth <jayatheerthkulkarni2005@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Make pull.c match the structural conventions
Date: Fri, 12 Dec 2025 13:50:18 +0900 [thread overview]
Message-ID: <xmqqikeccnhx.fsf@gitster.g> (raw)
In-Reply-To: <20251212020930.11654-1-jayatheerthkulkarni2005@gmail.com> (K. Jayatheerth's message of "Fri, 12 Dec 2025 07:39:30 +0530")
K Jayatheerth <jayatheerthkulkarni2005@gmail•com> writes:
> 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.
next prev parent reply other threads:[~2025-12-12 4:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-12 2:09 [PATCH] Make pull.c match the structural conventions K Jayatheerth
2025-12-12 4:50 ` Junio C Hamano [this message]
2025-12-12 7:44 ` [PATCH v2] pull: move options[] array into function scope K Jayatheerth
2025-12-12 14:08 ` [PATCH] Make pull.c match the structural conventions Kristoffer Haugsbakk
2025-12-12 14:48 ` JAYATHEERTH K
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqikeccnhx.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jayatheerthkulkarni2005@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox