Re: [PATCH 1/2] sequencer: trivial fix
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- May 29, 2013, 13:54 UTC
- Message-ID
- <CACsJy8Br1gigKL2GHQxC-9X8nizXzWcBysVuCHCGh1F3DiGp8Q@mail.gmail.com>
- In-Reply-To
- <CAMP44s0vaPUmvS8OzZ_4UR5XmKK-fyU4nE47Djjr215zkgpRFg@mail.gmail.com>
On Wed, May 29, 2013 at 8:46 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:
Show 28 quoted lines
> On Wed, May 29, 2013 at 8:42 AM, Duy Nguyen <pclouds@gmail.com> wrote: >> On Wed, May 29, 2013 at 8:34 PM, Felipe Contreras >> <felipe.contreras@gmail.com> wrote: >>> On Wed, May 29, 2013 at 8:25 AM, Duy Nguyen <pclouds@gmail.com> wrote: >>>> On Mon, May 27, 2013 at 11:52 PM, Felipe Contreras >>>> <felipe.contreras@gmail.com> wrote: >>>>> We should free objects before leaving. >>>>> >>>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> >>>> >>>> Micronit: perhaps you should move the "free obejcts before leaving" >>>> (in do_pick_commit) to the subject instead of "trivial fix", which >>>> adds no value to the patch. >>> >>> Perhaps. I prefer it this way because it's really a trivial fix not >>> really worth much time thinking about it. So when somebody is browsing >>> the history they can happily skip this one. The time save by not >>> reading I think adds more value than any succinct description that >>> would force each and every patch-reviewer/history-reader to read it. >> >> Some time from now, assume a ridiculus case when this function grows >> more complex and somebody wonders what the "leave" label is for, "git >> log --oneline -Slabel:" showing "trivial fix" would not help much. > > Fortunately that's not the main use-case, and for that single instance > that probably will never happen, I think it's not too much to ask to > this hypothetical developer to remove the --oneline, or copy-paste the > SHA-1 and take a peek. He would probably need to do that anyway.
And the "time saving by not reading" is also hypothetical. But I won't continue this discussion. -- Duy