On Sun, Oct 30 2022, Taylor Blau wrote: > On Sun, Oct 30, 2022 at 12:58:13PM +0100, René Scharfe wrote: >> > Changes since v1: >> > - Do the return value fix earlier; it was only an afterthought before. >> > Keep the colon (no "while at it, ..."). >> > - Break out the xstrdup(), oid_to_hex_r() and C99 cleanups. >> > - Convert tricky string arrays before strvecs because Ævar didn't like >> > the opposite order. >> > - Extend the example code in tmp-objdir.h so it still only requires >> > "cmd". >> >> Forgot one: >> - Fix grammar error in run-command.h added by the series in a comment >> that goes away at the end. >> >> René > > Thanks, replaced. This round is looking good to me, though let's hear > from ÆVar before we start merging this down. This LGTM. I don't think any of these are worth a re-roll, but just to clarify in reply to the comment in the v2 CL: * I thought the C99-specific stuff, free() etc. might be worth splitting out[1], but not to the extent of churning through first moving the existing API use around, and then converting it to run_command(). I.e. v1 had e.g. the am.c change as part of the large 3/8. Here there's 3/12, and we then re-visit that same caller in 12/12. I think that part of the v2 was better in v1. I.e. maybe split them out, but to the extent that it needs special explanation we could have just done it in one go with some explanation.. * I suggested just squashing what's now the v2's 08-11/12 into the respective commits that represented the last API user, or if they're worth splitting out do split-out with that last API user converted (and also removing the run-commit.h flags that went unused then, if applicable). Anyway, LGTM, and the stuff that was odd in the end state (e.g. 02/12) is now fixed. Thanks both! https://lore.kernel.org/git/Y11%2F2hyApN5NLruq@nand.local/T/#md2b7af02b1af34fa118779d3e1f4444604f95cb9
