All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Frédéric Heitzmann" <frederic.heitzmann@gmail.com>
To: Eric Wong <normalperson@yhbt.net>
Cc: git@vger.kernel.org, gitster@pobox.com, jaysoffian@gmail.com
Subject: Re: [PATCH 1/2] git svn dcommit: new option --interactive.
Date: Wed, 07 Sep 2011 22:02:36 +0200	[thread overview]
Message-ID: <4E67CDDC.1020305@gmail.com> (raw)
In-Reply-To: <20110906202601.GA11668@dcvr.yhbt.net>



Le 06/09/2011 22:26, Eric Wong a écrit :
> Frédéric Heitzmann<frederic.heitzmann@gmail.com>  wrote:
>> Allow the user to check the patch set before it is commited to SNV. It is then
>> possible to accept/discard one patch, accept all, or quit.
>>
>> This interactive mode is similar with 'git send email' behaviour. However,
>> 'git svn dcommit' returns as soon as one patch is discarded.
>>
>> Part of the code was taken from git-send-email.perl
>> Thanks-to: Eric Wong<normalperson@yhbt.net>  for the initial idea.
>> Signed-off-by: Frédéric Heitzmann<frederic.heitzmann@gmail.com>
> I agree with this feature, a few comments inline.
>
>>   I would have preferred not duplicating the code snippets taken from
>>   git-send-email ('ask' function, Term related code, ...) but I preferred not
>>   to spoil Git.pm with it.
>>   Any comment on a better way to factor perl code would be appreciated.
> We should put this into Git.pm at some point.
> (Somebody should refactor git-svn.perl into separate files too... :x)
>
>>   Documentation/git-svn.txt |    8 +++++
>>   git-svn.perl              |   71 ++++++++++++++++++++++++++++++++++++++++++++-
>>   2 files changed, 78 insertions(+), 1 deletions(-)
> Tests and feature should be the same patch
>> +	return defined $default ? $default : undef
>> +		unless defined $term->IN and defined fileno($term->IN) and
>> +		       defined $term->OUT and defined fileno($term->OUT);
> Things to make life easier for (mainly) C programmers:
>
> * Use C-style "&&" and "||" for conditionals.  "and" and "or" are lower
>    precedence and better used for control flow (see perlop(1) manpage).
>
> * Also, use parentheses for defined(foo) to disambiguate multiple
>    conditions/statements.
>
My fault : I copied-pasted the 'ask' function from git-send-email.
Even if I rewrite it a litlle, it should not prevent anyone to mutalize 
some code into Git.pm.
And, indeed, it will improve readability.

I wait a few days to see if anyone else has some comments and I send a 
V2 patch serie.

Thanks for reviewing.

--
Fred

  reply	other threads:[~2011-09-07 20:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-04 19:21 [PATCH] git svn dcommit : new option --interactive Frédéric Heitzmann
2011-09-04 19:21 ` [PATCH 1/2] git svn dcommit: " Frédéric Heitzmann
2011-09-06 20:26   ` Eric Wong
2011-09-07 20:02     ` Frédéric Heitzmann [this message]
2011-09-04 19:21 ` [PATCH 2/2] git svn dcommit: add a test serie for 'git svn dcommit --interactive' Frédéric Heitzmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E67CDDC.1020305@gmail.com \
    --to=frederic.heitzmann@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jaysoffian@gmail.com \
    --cc=normalperson@yhbt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.