On Tue, Feb 16, 2021 at 12:11:08AM +0100, Ævar Arnfjörð Bjarmason wrote: > > I split this into two patches: one to move the trace2_cmd_mode() calls > > around, and another to replace the final 'die()' with the usage text. > > Thanks for picking it up. Of course. This has been quite a fun digression :-). > > Like I said in my review of your patches to the commit-graph builtin > > here: > > > > https://lore.kernel.org/git/YCrDGhIq7kU57p1s@nand.local/ > > > > I don't find the 'if (argc && ...)' style more readable, so the second > > patch looks like this instead: > > *Nod* FWIW (and this is getting way to nit-y) I don't disagree with you > about the "argc &&" being not very readable, > > I just lean more on the side of getting rid of duplicate branches, > you'll still need the if (!argc) usage(...) case above without that > pattern, or some replacement for it. > > But we can have our cake (not re-check argc all the time) and eat it too > (not copy/paste usage_with_options()). Isn't it beautiful? Heh; I'm not sure that I'd call adding a goto "beautiful", but I actually do find this one more readable. I dunno, honestly, I'm happy to squash it in to the last commit on top, but honestly I don't really care strongly one way or another ;). > > Is it OK if I use your Signed-off-by on both of those two new patches? > > Yes please, should have included it to begin with. Thanks, and no worries. Thanks, Taylor
