Re: [PATCH 2/4] git-rebase: add keep_empty flag

[Neil Horman <nhorman@tuxdriver.com>]

On Fri, Mar 30, 2012 at 01:43:22PM -0700, Junio C Hamano wrote:
> Neil Horman <nhorman@tuxdriver.com> writes:
> 
> > Add a command line switch to git-rebase to allow a user the ability to specify
> > that they want to keep any commits in a series that are empty.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Jeff King <peff@peff.net>
> > CC: Phil Hord <phil.hord@gmail.com>
> > CC: Junio C Hamano <gitster@pobox.com>
> 
> The same comments on Cc: apply to all of your patches.
> 
Roger that.

> >  Documentation/git-rebase.txt |    6 ++++++
> >  git-rebase.sh                |    5 +++++
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > index 504945c..9717d3e 100644
> > --- a/Documentation/git-rebase.txt
> > +++ b/Documentation/git-rebase.txt
> > @@ -238,6 +238,12 @@ leave out at most one of A and B, in which case it defaults to HEAD.
> >  	will be reset to where it was when the rebase operation was
> >  	started.
> >  
> > +--keep-empty::
> > +	Informs git-rebase that comits which are empty should not be
> > +	automatically removed.  This is at times useful when empty commits
> > +	are used to hold developer information and notes, but contain no real
> > +	code changes
> > +
> 
> Unlike "cherry-pick", I think "--keep-empty" is a better name for the
> option than "--allow-empty" in this context.  The difference is that from
> the end-user's point of view, cherry-pick _replays_ commits that exist
> elsewhere, and you are allowing the command to replay empty ones as well,
> while rebase _rebuilds_ commits on the same branch, and you are telling
> the command to keep empty ones.
> 
> "... which are empty should not be removed" is a bit of double-negation,
> though.  Perhaps
> 
> 	--keep-empty::
> 		Keep the commits that do not change anything from its
> 		parents in the result.  This is at times useful when empty
> 		commits are used to hold developer information and notes
> 		without having any real changes.
> 
> But as I rephrased the first part, the last line may have become redundant
> and could safely be removed.
> 
Ack, I can make the above changes.

> The patch does not seem to do anything other than accepting and silently
> ignoring the option, though.
> 
It does, as the use of the option is pushed into the type specific rebase
scripts.  I probably should have rolled this patch in with one of those. I'll
squash it in when I repost.