From: "Shawn O. Pearce" <spearce@spearce.org>
To: Sverre Rabbelier <srabbelier@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Junio C Hamano <gitster@pobox.com>,
Git List <git@vger.kernel.org>
Subject: Re: [PATCH 1/4] fast-import: put option parsing code in seperate functions
Date: Thu, 13 Aug 2009 07:40:07 -0700 [thread overview]
Message-ID: <20090813144007.GJ1033@spearce.org> (raw)
In-Reply-To: <alpine.DEB.1.00.0908131208400.7429@intel-tinevez-2-302>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 12 Aug 2009, Sverre Rabbelier wrote:
>
> > +static void option_export_marks(const char *marks)
> > +{
> > + struct strbuf buf = STRBUF_INIT;
> > + strbuf_addstr(&buf, marks);
> > + mark_file = strbuf_detach(&buf, NULL);
> > +}
>
> Heh, this is a pretty convoluted way to write
>
> mark_file = xstrdup(marks);
>
> ;-)
Agreed.
> > +static void option_force()
> > +{
> > + force_update = 1;
> > +}
>
> I'm not sure that I would put these simple assignments in separate
> functions, but that's certainly up to you!
Oh, good point. Yes, please don't do that, please just write these
directly into the option parser.
Aside from these remarks, this patch looks good to me.
--
Shawn.
prev parent reply other threads:[~2009-08-13 14:40 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-13 5:09 [PATCH 0/4] fast-import: add a new option command Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 1/4] fast-import: put option parsing code in seperate functions Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 2/4] fast-import: define a new option command Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 3/4] fast-import: add " Sverre Rabbelier
2009-08-13 5:09 ` [PATCH 4/4] fast-import: test the new " Sverre Rabbelier
2009-08-13 14:45 ` [PATCH 3/4] fast-import: add " Shawn O. Pearce
2009-08-13 14:43 ` [PATCH 2/4] fast-import: define a new " Shawn O. Pearce
2009-08-13 14:56 ` Johannes Schindelin
2009-08-13 15:04 ` Shawn O. Pearce
[not found] ` <fabb9a1e0908130812s297ccfc6vd6b746daf1dcc69a@mail.gmail.com>
2009-08-13 15:24 ` Shawn O. Pearce
2009-08-13 16:26 ` Sverre Rabbelier
2009-08-13 17:07 ` Johannes Schindelin
2009-08-13 17:09 ` Sverre Rabbelier
2009-08-13 17:25 ` Shawn O. Pearce
2009-08-13 17:28 ` Sverre Rabbelier
2009-08-13 17:41 ` Shawn O. Pearce
2009-08-13 17:44 ` Sverre Rabbelier
2009-08-13 17:52 ` Shawn O. Pearce
2009-08-13 21:51 ` Johannes Schindelin
2009-08-13 22:01 ` Sverre Rabbelier
2009-08-13 22:12 ` Shawn O. Pearce
2009-08-13 22:17 ` Sverre Rabbelier
2009-08-13 19:26 ` Junio C Hamano
2009-08-13 20:01 ` Sverre Rabbelier
2009-08-13 20:42 ` Junio C Hamano
2009-08-13 21:14 ` Sverre Rabbelier
2009-11-23 17:39 ` Sverre Rabbelier
2009-11-23 20:50 ` Shawn O. Pearce
2009-08-13 10:19 ` [PATCH 1/4] fast-import: put option parsing code in seperate functions Johannes Schindelin
2009-08-13 14:40 ` Shawn O. Pearce [this message]
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=20090813144007.GJ1033@spearce.org \
--to=spearce@spearce.org \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox