On Thu, Sep 12, 2013 at 11:44:30AM +0200, Matthieu Moy wrote: > That is clean, but a bit long and it is essentially duplicated between > status and commit. I went another way: put all the similar code in a > common function status_init_config: > > static void status_init_config(struct wt_status *s, config_fn_t fn) > { > wt_status_prepare(s); > gitmodules_config(); > git_config(git_status_config, s); > determine_whence(s); > s->hints = advice_status_hints; /* must come after git_config() */ > } s/git_status_config/fn/, I assume. > We could split the git_config call, but that would not bring much > benefit IMHO. In any case, it can be done very simply on top of my patch > if needed later, as there is now only one call site for git_config. Yeah, I think that is fine. The other cleanup may or may not be worth it, but should not be a blocker to your patch. With what you suggest above, you are certainly not making anything worse with respect to the code organization. Thanks. -Peff -- 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