Attention is currently required from: Angel Pons, Anastasia Klimchuk, Nikolai Artemiev.

View Change

1 comment:

• Patchset:

  • Patch Set #2:

    Thanks for reply! :)

    I'm not sure if I mentioned this somewhere already, the topic seems familiar. I kind of see this extract feature as CLI code.

    Sorry if I missed that! I don't remember, but maybe because extract feature was upstreamed earlier when I just started and I wasn't fully aware of all everything that going on.

    Don't worry, if we discussed this somewhere else already, I lost track
    of it anyway.

    My reasoning for this patch chain was to align with larger movement to make cli a libflashrom user, which means libflashrom is used from everywhere and needs to be tested.

    Yes, I got that :) we need to adapt libflashrom API for this. Making
    existing functions public isn't always the best way, though.

    
    > So far, we have kept libflashrom free from file i/o and some target environments just don't support it

    Maybe I am missing something, but I don't see `prepare_layout_for_extraction` doing file i/o at the moment? It is populating file names, which is prep step for i/o, but the i/o itself is done next in read operation.

    That's right. But for prepare_layout_for_extraction() to be useful in the
    libflashrom API, we'd need another function there that probably does what
    write_buf_to_include_args() currently does. And that's doing file i/o.
    Also, the description in your next patch says "Fill in file name". So I'd
    say it's part of a file i/o concept.

    It's why I generally prefer to discuss the coarse direction of a topic
    on the mailing list first. Here on Gerrit, people are often focused to
    bring patches in step-by-step, but sometimes the bigger picture is only
    visible at the end of a patch train. It can be frustrating for both authors
    and reviewers to see that the effort on prior reviews doesn't pan out. Or
    worse, they might want to merge something that they consider wrong just
    because they put all the effort into it.

    Maybe `prepare_layout_for_extraction` can be renamed into something more specific?

    I would much prefer to discuss first if the whole topic belongs into
    libflashrom or the CLI.

    I agree with keeping libflashrom free from file i/o.

To view, visit change 62554. To unsubscribe, or for help writing mail filters, visit settings.