On Fri, Apr 23, 2021 at 6:43 PM Luke Shumaker <lukeshu@xxxxxxxxxxx> wrote: > On Fri, 23 Apr 2021 14:23:18 -0600, Eric Sunshine wrote: > > What is the purpose of this change? Does some subsequent commit depend > > upon this or is it just a personal preference? The commit message > > explains the "what" of the change but not the "why". > > Dropping the commit would surely cause me much trouble with rebasing > both the subsequent commits in this patchset and the commits I haven't > yet submitted. But I don't think they "depend" on it. > > I guess it is personal preference... [...] That's fine. I wasn't suggesting removing the commit but was interested in having the commit message explain the reason for the change, which is often the most important part of the commit message to convince readers (reviewers) that the patch isn't just needless noise. Obviously, if changes in subsequent patches become easier by making this change, then that's easier to explain than something which is mere personal preference -- which isn't to say that such preference isn't important, especially if you're going to be living in the code for a while. > In this specific case, it's also moving the `set -- -h`, the `git > rev-parse --parseopt`, and the `. git-sh-setup` to be closer to all > the rest of the argument parsing, which is a readability win on its > own, IMO. If you happen to re-roll, perhaps update the commit message to include a small bit of your explanation (which I've mostly snipped here). The last bit about moving `set -- -h` and whatnot closer to the rest of the argument parsing -- basically improving encapsulation -- may be justification enough for readers.
