On Tue, Mar 12, 2013 at 03:04:56PM -0400, Jeff King wrote:
> On Tue, Mar 12, 2013 at 04:44:35PM +0100, Heiko Voigt wrote:
> > > Can we throw in a comment at the top here with the expected usage? In
> > > particular, do_config_from is expecting the caller to have filled in
> > > certain fields (at this point, top->f and top->name), but there is
> > > nothing to make that clear.
> > Of course. Will do that in the next iteration. How about I squash this in:
> > [...]
> > +/* The fields data, name and the source specific callbacks of top need
> > + * to be initialized before calling this function.
> > + */
> > static int do_config_from_source(struct config_source *top, config_fn_t
> > fn, voi
> I think that is OK, but it may be even better to list the fields by
> name. Also, our multi-line comment style is:
> * Multi-line comment.
Ok will do both.
> > I would add that to the third patch:
> > config: make parsing stack struct independent from actual data source
> > because that contains the final modification to config_file/config_source.
> It does not matter to the end result, but I find it helps with reviewing
> when the comment is added along with the function, and then expanded as
> the function is changed. It helps to understand the effects of later
> patches if they need to tweak comments.
To make the series more clear to others who read it later, I will add
the comment from the beginning.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html