[PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread Felipe Contreras
We should free objects before leaving.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 builtin/sequencer.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/sequencer.c b/builtin/sequencer.c
index b2c8c94..23b01b7 100644
--- a/builtin/sequencer.c
+++ b/builtin/sequencer.c
@@ -539,8 +539,10 @@ static int do_pick_commit(struct commit *commit, struct 
replay_opts *opts)
}
 
allow = allow_empty(opts, commit);
-   if (allow  0)
-   return allow;
+   if (allow  0) {
+   res = allow;
+   goto leave;
+   }
if (!opts-no_commit)
res = run_git_commit(defmsg, opts, allow);
 
-- 
1.8.3.698.g079b096

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread SZEDER Gábor
On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
 We should free objects before leaving.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com

A shortlog-friendlier subject could be: sequencer: free objects
before leaving.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread Felipe Contreras
On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
 We should free objects before leaving.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com

 A shortlog-friendlier subject could be: sequencer: free objects
 before leaving.

I already defended my rationale for this succinct commit message:

http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread SZEDER Gábor
On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
 On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
  We should free objects before leaving.
 
  Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 
  A shortlog-friendlier subject could be: sequencer: free objects
  before leaving.
 
 I already defended my rationale for this succinct commit message:
 
 http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610

Your arguments were unconvincing.  The mere fact that I raised this
issue unbeknownst to the earlier posting clearly shows that there's
demand for descriptive subjects.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread John Keeping
On Sun, Jun 09, 2013 at 07:33:42PM +0200, SZEDER Gábor wrote:
 On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
  On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
   On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
   We should free objects before leaving.
  
   Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  
   A shortlog-friendlier subject could be: sequencer: free objects
   before leaving.
  
  I already defended my rationale for this succinct commit message:
  
  http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610
 
 Your arguments were unconvincing.  The mere fact that I raised this
 issue unbeknownst to the earlier posting clearly shows that there's
 demand for descriptive subjects.

Not to mention that with your subject no body is needed, making the
overall message more succinct.

When reading a log, as soon as I see trivial I become suspicious that
someone is trying to cover something up, much like left as an exercise
for the reader.  If the subject says fix memory leak then it's
obvious what the patch is meant to do, and when there is no subtlety to
be explained (as there isn't in this patch) there is no need for a body.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread Felipe Contreras
On Sun, Jun 9, 2013 at 12:33 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
 On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
  We should free objects before leaving.
 
  Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 
  A shortlog-friendlier subject could be: sequencer: free objects
  before leaving.

 I already defended my rationale for this succinct commit message:

 http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610

 Your arguments were unconvincing.

That's your opinion. The commit message I write and send is my decision.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread SZEDER Gábor
On Sun, Jun 09, 2013 at 12:47:16PM -0500, Felipe Contreras wrote:
 On Sun, Jun 9, 2013 at 12:33 PM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
  On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
   On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
   We should free objects before leaving.
  
   Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  
   A shortlog-friendlier subject could be: sequencer: free objects
   before leaving.
 
  I already defended my rationale for this succinct commit message:
 
  http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610
 
  Your arguments were unconvincing.
 
 That's your opinion.

And Duy's.  And John's, too, apparently.

 The commit message I write and send is my decision.

It's always a pleasure to work with you.

Guess I should've known better.


Bye,
Gábor

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread Felipe Contreras
On Sun, Jun 9, 2013 at 12:37 PM, John Keeping j...@keeping.me.uk wrote:
 On Sun, Jun 09, 2013 at 07:33:42PM +0200, SZEDER Gábor wrote:
 On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
  On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
   On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
   We should free objects before leaving.
  
   Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  
   A shortlog-friendlier subject could be: sequencer: free objects
   before leaving.
 
  I already defended my rationale for this succinct commit message:
 
  http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610

 Your arguments were unconvincing.  The mere fact that I raised this
 issue unbeknownst to the earlier posting clearly shows that there's
 demand for descriptive subjects.

 Not to mention that with your subject no body is needed, making the
 overall message more succinct.

It's not succinct at all, because there's no short and quick
description of what the patch actually is; a trivial fix.

 When reading a log, as soon as I see trivial I become suspicious that
 someone is trying to cover something up, much like left as an exercise
 for the reader.  If the subject says fix memory leak then it's
 obvious what the patch is meant to do, and when there is no subtlety to
 be explained (as there isn't in this patch) there is no need for a body.

You are not a rational person then. The commit message has absolutely
no bearing on the quality of the code. If you are less suspicious of a
commit message that says fix memory leak, you are being completely
biased.

Whether the commit message says fix memory leak, or trivial fix,
or foobar, the code might still be doing something wrong, and you
can't decide that until you look at the code.

If you don't care about the code, but still want to know what the
patch is doing, then you can look at the whole commit message, and We
should free objects before leaving. explains that perfectly.

For the people that only read the summary, the vast majority of them
need to know what this patch is, not what it does, and when they see
trivial fix they most likely can skip it.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread Felipe Contreras
On Sun, Jun 9, 2013 at 12:53 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Sun, Jun 09, 2013 at 12:47:16PM -0500, Felipe Contreras wrote:
 On Sun, Jun 9, 2013 at 12:33 PM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
  On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
   On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
   We should free objects before leaving.
  
   Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  
   A shortlog-friendlier subject could be: sequencer: free objects
   before leaving.
 
  I already defended my rationale for this succinct commit message:
 
  http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610
 
  Your arguments were unconvincing.

 That's your opinion.

 And Duy's.  And John's, too, apparently.

And it could the opinion of a thousand more people, it doesn't make it
anything more than an opinion. To think anything else is to fall in
argumentum ad populum.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread John Keeping
On Sun, Jun 09, 2013 at 12:53:38PM -0500, Felipe Contreras wrote:
 On Sun, Jun 9, 2013 at 12:37 PM, John Keeping j...@keeping.me.uk wrote:
  On Sun, Jun 09, 2013 at 07:33:42PM +0200, SZEDER Gábor wrote:
  On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
   On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote:
On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
We should free objects before leaving.
   
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
   
A shortlog-friendlier subject could be: sequencer: free objects
before leaving.
  
   I already defended my rationale for this succinct commit message:
  
   http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610
 
  Your arguments were unconvincing.  The mere fact that I raised this
  issue unbeknownst to the earlier posting clearly shows that there's
  demand for descriptive subjects.
 
  Not to mention that with your subject no body is needed, making the
  overall message more succinct.
 
 It's not succinct at all, because there's no short and quick
 description of what the patch actually is; a trivial fix.

Is it not equally succinct to say fix memory leak?

  When reading a log, as soon as I see trivial I become suspicious that
  someone is trying to cover something up, much like left as an exercise
  for the reader.  If the subject says fix memory leak then it's
  obvious what the patch is meant to do, and when there is no subtlety to
  be explained (as there isn't in this patch) there is no need for a body.
 
 You are not a rational person then. The commit message has absolutely
 no bearing on the quality of the code. If you are less suspicious of a
 commit message that says fix memory leak, you are being completely
 biased.

 Whether the commit message says fix memory leak, or trivial fix,
 or foobar, the code might still be doing something wrong, and you
 can't decide that until you look at the code.

I have a certain level of trust that commit summaries in git.git will be
accurate.  If I want to know what has changed, then fix memory leak
implies no functional change; if I see trivial fix then how do I
know what that is?  It could be a whitespace change, a fix to a memory
leak, a typo correction, a change to a separator in a message shown to
the user, or even a small change to corner case behaviour.

 If you don't care about the code, but still want to know what the
 patch is doing, then you can look at the whole commit message, and We
 should free objects before leaving. explains that perfectly.

The short message is what appears in What's Cooking, why should I need
to break out of my mail client to find out what it means?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread SZEDER Gábor
On Sun, Jun 09, 2013 at 02:11:18PM -0500, Felipe Contreras wrote:
 Perhaps trivial memory leak fix would be better.

That's fine with me, thanks.

If only we could have got there like 10 emails ago.


Gábor

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 10/45] sequencer: trivial fix

2013-06-09 Thread Philip Oakley

From: Felipe Contreras felipe.contre...@gmail.com
Sent: Sunday, June 09, 2013 6:23 PM
On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de 
wrote:

On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:

We should free objects before leaving.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com


A shortlog-friendlier subject could be: sequencer: free objects
before leaving.


I already defended my rationale for this succinct commit message:

http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610

--
Felipe Contreras


I think I'd prefer a mixture of both.

sequencer: trivial fix: free objects before leaving.
This gives the best of both worlds in that the 'triviality' is plainly 
there to see, and so is the type of triviality, just in case it has some 
un-noticed side effect that someone is looking for at a leter date


Same goes for build: trivial cleanup: don't repeat prerequisites 
[PATCH v4 03/45]


All the best,

Philip


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html