Re: [PATCH 1/4] fast-import: put option parsing code in seperate functions
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Aug 13, 2009, 14:40 UTC
- Message-ID
- <20090813144007.GJ1033@spearce.org>
- In-Reply-To
- <alpine.DEB.1.00.0908131208400.7429@intel-tinevez-2-302>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 14 quoted lines
> 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.
Show 7 quoted lines
> > +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.