From: Junio C Hamano Date: Wed, 19 Oct 2022 15:43:43 GMT Subject: Re: [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt() Message-ID: In-Reply-To: Jeff King writes: > I do think Junio saying "consensus" may have been premature. I expressed > my opinion and he agreed, but I think that is as far as it got. :) Maybe. This is a tangent, but as far as I am concerned, when Réne writes something that looks to me very straight-forward and correct, and it passes your taste buds, then that is enough consensus to move ahead. As Linus often said and I concur, some people got good taste, that is hard to quantify and probably hard to teach, and there are a handful folks here with good taste. And when two who have demonstrated they are with good taste agrees, that is good enough to me. >> I don't see how *_l_opt() is particularly error prone, I just had a >> stupid think-o in v1 of this, but that if/else if bug is something that >> could have snuck in with run_command() given the same stupidity :) > > I don't think it's error-prone. It just seems like it complicates an API > for little gain, and causes us to have a lot of boilerplate mapping > RUN_* flags into cmd.* fields. True. run_command() needs the RUN_* flags twiddling, too, so it is not a point against _l_opt() variant. What I see as its biggest problem is it is a bit too rigid for many of the current callers that use _v_opt() to replace them, and can easily tempt developers to write almost-duplicate _l_opt() calls, leading to an easily avoidable bug like we saw in the if/else if/else cascade. Thanks.