From: Kristoffer Haugsbakk Date: Fri, 12 Dec 2025 14:08:55 GMT Subject: Re: [PATCH] Make pull.c match the structural conventions Message-ID: <52483794-bdba-44b8-9222-761184ecea95@app.fastmail.com> In-Reply-To: On Fri, Dec 12, 2025, at 05:50, Junio C Hamano wrote: > K Jayatheerth 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’ve seen some commit messages in the last few months that have too many adjectives. I’ve never seen that style before. > > 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.