04/01/2024 13:34, Luca Vizzarro: > On 04/01/2024 10:52, Thomas Monjalon wrote: > >> DTS needs to know which nodes to connect to and what hardware to use on > >> those nodes. > >> -Once that's configured, DTS needs a DPDK tarball and it's ready to run. > >> +Once that's configured, DTS needs a DPDK tarball or a git ref ID and it's > >> ready to run. > > > > That's assuming DTS is compiling DPDK. > > We may want to provide an already compiled DPDK to DTS. > > Yes, that is correct. At the current state, DTS is always compiled from > source though, so it may be reasonable to leave it as it is until this > feature may be implemented. Nonetheless, my change just informs the user > of the (already implemented) feature that uses `git archive` from the > local repository to create a tarball. A sensible change would be to add > this explanation I have just given, but it is a technicality and it > won't really make a difference to the user.
Yes I would like to make it clear in this doc that DTS is compiling DPDK. Please could you change to something like "DTS needs a DPDK tarball or a git ref ID to compile" ? I hope we will change it later to allow external compilation. > >> + (dts-py3.10) $ ./main.py --help > > > > Why adding this line? > > Just running `./main.py` will just throw a confusing error to the user. > I am in the process of sorting this out as it is misleading and not > helpful. Specifying the line in this case just hints to the user on the > origin of that help/usage document. Yes would be good to have a message to help the user instead of a confusing error. > > Should we remove the shell prefix referring to a specific Python version? > > I have purposely left the prefix to indicate that we are in a Poetry > shell environment, as that is a pre-requisite to run DTS. So more of an > implicit reminder. The Python version specified is in line with the > minimum requirement of DTS. OK > > In general it is better to avoid long lines, and split after a punctation. > > I think we should take the habit to always go to the next line after the > > end of a sentence. > > I left the output of `--help` under a code block as it is originally > printed in the console. Could surely amend it in the docs to be easier > to read, but the user could as easily print it themselves in their own > terminal in the comfort of their own environment. I was not referring to the console output. Maybe I misunderstood it. For the doc sentences, please try to split sentences on different lines. > >> - [DTS_OUTPUT_DIR] Output directory where dts > >> logs and results are > >> - saved. (default: output) > >> + [DTS_OUTPUT_DIR] Output directory where dts > >> logs and results are saved. > > > > dts -> DTS > > As above. The output of `--help` only changed as a result of not being > updated before in parallel with code changes. Consistently this is what > the user would see right now. It may or may not be a good idea to update > this whenever changed in the future. I did not understand it is part of the help message. > Nonetheless, I am keen to update the code as part of this patch to > resolve your comments. Yes please update the code for this small wording fix. > > Please don't add compilation configuration for now, > > I would like to work on the schema first. > > This is mostly imported from the old DTS and needs to be rethink. > > While I understand the concern on wanting to rework the schema, which is > a great point you make, it may be reasonable to provide something useful > to close the existing documentation gap. And incrementally updating from > there. If there is no realistic timeline set in place for a schema > rework, it may just be better to have something rather than nothing. And > certainly it would not be very useful to upstream a partial documentation. I don't know. I have big doubts about the current schema. I will review it with your doc patches. Can you please split this patch in 2 so that the schema doc is in a different patch? > Thank you a lot for your review! You have made some good points which > open up new potential tasks to add to the pipeline.