Thread (15 messages) flat view 15 messages, 4 authors, 2021-08-24

Re: [PATCH v2 0/4] test-tool: split up "read-cache" tool

From: Taylor Blau <hidden>
Date: 2021-08-24 22:19:58

On Tue, Aug 24, 2021 at 11:15:21AM +0200, Ævar Arnfjörð Bjarmason wrote:
A re-roll addressing the feedback v1 got, see
https://lore.kernel.org/git/cover-0.4-0000000000-20210607T115454Z-avarab@gmail.com (local)

I think the gist of that feedback from Emily was that v1 could be
understood as aiming to optimize the perf test, but the main reason
I'm doing this split up is to make the code easier to read. The
changed commit message summarizes both goals.

I also changed some nits in the code I spotted myself, e.g. "argc > 0"
checks to just "argc", and a simpler way of providing the "cnt"
default.
Playing devil's advocate for a moment: I think that the current
implementation is somewhat easier to read as it lives in a single file,
and makes clear that each option implies different behavior through the
body of the loop.

If we're looking for things to clean up, I do like the conversion to the
parse-options API instead of reading argv ourselves, but probably
otherwise prefer the code as-is instead of split across many files.

But I may be in the minority, and there may be others who do find the
split-up version easier to grok.

Thanks,
Taylor
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help