All of lore.kernel.org
 help / color / mirror / Atom feed
From: Erik Faye-Lund <kusmabite@gmail.com>
To: Ramkumar Ramachandra <artagnon@gmail.com>
Cc: Git List <git@vger.kernel.org>,
	Jonathan Nieder <jrnieder@gmail.com>,
	David Barr <david.barr@cordelta.com>,
	Sverre Rabbelier <srabbelier@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/3] vcs-svn: Introduce svnload, a dumpfile producer
Date: Wed, 2 Feb 2011 13:43:06 +0100	[thread overview]
Message-ID: <AANLkTi=sQtaWc0C5tqjgmAqCoLupKg0fPhEd+-Y5kx=x@mail.gmail.com> (raw)
In-Reply-To: <20110202025320.GB11339@kytes>

On Wed, Feb 2, 2011 at 3:53 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> Hi Erik,
>
> Erik Faye-Lund writes:
>> A very superficial review, because I don't have much time, and don't
>> know the surrounding code well. Sorry about that.
>
> Thanks. I have to often switch back and fourth between GNU-style (for
> Subversion) and Linux-style (for Git), so please bear with my silly
> style errors.
>

That's completely fair :)

>> On Tue, Feb 1, 2011 at 3:26 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
>> > +       if (!val) die("Malformed author line");
>> > +       if (!(tz_off = strrchr(val, ' '))) goto error;
>> > +       *tz_off++ = '\0';
>> > +       if (!(t = strrchr(val, ' '))) goto error;
>>
>> style: use
>> "if (x)
>>       do_stuff();"
>> instead of
>> "if (x) do_stuff();"
>
> This was intentional -- I'm only using it when do_stuff() is 'goto
> error'. I thought it looked nicer that way.
>

This is still not the style we use in git:

$ git grep "if (" | grep goto | wc -l
      0
$ git grep -B1 "if (" | grep goto | wc -l
     33

  reply	other threads:[~2011-02-02 12:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-01 14:26 [PATCH v3 0/3] Towards a Git-to-SVN bridge Ramkumar Ramachandra
2011-02-01 14:26 ` [PATCH 1/3] vcs-svn: Introduce svnload, a dumpfile producer Ramkumar Ramachandra
2011-02-01 14:46   ` Erik Faye-Lund
2011-02-02  2:53     ` Ramkumar Ramachandra
2011-02-02 12:43       ` Erik Faye-Lund [this message]
2011-02-01 14:26 ` [PATCH 2/3] t9010-svn-fi: Add tests for svn-fi Ramkumar Ramachandra
2011-02-01 18:58   ` Jonathan Nieder
2011-02-02  2:49     ` Ramkumar Ramachandra
2011-02-02  3:18       ` Jonathan Nieder
2011-02-01 14:26 ` [PATCH 3/3] vcs-svn: Refactor dump_export code into dispatch table Ramkumar Ramachandra
2011-02-01 17:42   ` Jonathan Nieder
2011-02-01 21:29     ` Junio C Hamano
2011-02-02  2:56     ` Ramkumar Ramachandra

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='AANLkTi=sQtaWc0C5tqjgmAqCoLupKg0fPhEd+-Y5kx=x@mail.gmail.com' \
    --to=kusmabite@gmail.com \
    --cc=artagnon@gmail.com \
    --cc=david.barr@cordelta.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=srabbelier@gmail.com \
    /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.