Re: [PATCH v2 00/10] run-command API: add run_command_{l,sv}_opt()

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

 



On Wed, Oct 19, 2022 at 08:43:43AM -0700, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> 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 pretty much agree with that worldview, but I try hard not to assume
"my taste is good" when going into a technical discussion. If only
because I've embarrassed myself a sufficient number of times. :)

> >> 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.

Did you mean run_command_v() here? If so, then yes, it requires the
flags. But if we are going to drop it in favor of just run_command(),
then those flags go away (and moving to the _l() variant is a step in
the opposite direction).

-Peff



[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]

  Powered by Linux