Re: [PATCH 1/2] wt-status: turn advice_status_hints into a field of wt_status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]