Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html