From: René Scharfe Date: Sat, 31 Aug 2013 10:28:03 GMT Subject: Re: [PATCH] branch: use $curr_branch_short more Message-ID: <5221C533.1070109@web.de> In-Reply-To: Am 31.08.2013 11:22, schrieb Felipe Contreras: > On Sat, Aug 31, 2013 at 4:11 AM, René Scharfe 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. >> 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. >> 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. >> 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. René