Re: [PATCH] branch: use $curr_branch_short more
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Aug 31, 2013, 17:20 UTC
- Message-ID
- <CAMP44s39X2SsZC7f6=j=CX+9wky7YpiJUz3itCiqeLScD0TbNA@mail.gmail.com>
- In-Reply-To
- <5221C533.1070109@web.de>
On Sat, Aug 31, 2013 at 5:28 AM, René Scharfe <l.s.r@web.de> wrote:
Show 19 quoted lines
> Am 31.08.2013 11:22, schrieb Felipe Contreras: > >> On Sat, Aug 31, 2013 at 4:11 AM, René Scharfe <l.s.r@web.de> wrote: >> >>>> Subject: pull: trivial simplification >>>> >>>> With that summary, people would have an easier time figuring out if >>>> they need to read more about the patch or not. >>> >>> >>> >>> "trivial simplification" is too generic; we could have lots of them. >> >> >> No, we can have only one, otherwise it would say simplificationS. > > > I was too terse again, let me rephrase that: We could have lots of commits > that fit the same description if we used such a generic one.
Yes, but they are all trivial, and they all simply the code.
Show 15 quoted lines
>>> A summary should describe the change. >> >> >> You can never fully describe the change, only the diff does that. >> >> For example "use $curr_branch_short more" does not tell me anything >> about the extent of the changes, is it used in one more place? two? >> one hundred? Moreover, how exactly is it used more? Is some >> refactoring needed? >> >> And it still doesn't answer the most important question any summary >> should answer: why? Why use $curr_branch_short more? > > A summary doesn't have to contain lots of details. The what is important, > the why can be explained in the commit message.
A summary should contain as much information that would allow me to skip the commit message as possible.
If I can't tell from the summary if I can safely skip the commit message, the summary is not doing a good job.
"trivial simplification" explains the "what", and the "why" at the same time, and allows most people to skip the commit message, thus is a good summary.
Show 22 quoted lines
>>> Its low complexity can be derived from >>> it -- using an existing variable a bit more is not very exciting. >> >> >> You didn't say "a bit more" you said "more". And yes, the complexity >> can be derived from the summary, but not from this one. >> >>> But I wouldn't call that patch trivial because its correctness depends on >>> code outside of its shown context. >> >> >> Correctness is a separate question from triviality, and the >> correctness can only be assessed by looking at the actual patch. >> >> The patch can be both trivial and wrong. > > > Probably too terse again, let's say it differently: Only a patch whose > correctness can be judged without looking outside of the three lines of > context it includes qualifies as trivial in my book. The patch in question > is not trivial because you can't see the value of $curr_branch_short just by > looking at the diff.
Again, triviality and correctness are two separate different things. The patch is trivial even if you can't judge it's correctness.
To me, what you are describing is an obvious patch, not a trivial one. An obvious patch is so obvious that you can judge it's correctness easily by looking at the diff, a trivial one is of little importance.
Show 10 quoted lines
>>> The reason for the patch isn't mentioned explicitly. Perhaps it should >>> be. >>> I felt that using something that's already there instead of recreating it >>> is >>> motivation alone. >> >> >> Why? Because it simplifies the code. That's the real answer. > > I don't disagree.
Yet your commit message doesn't explain that anywhere.
-- Felipe Contreras