[PATCH] dts: improve documentation
Thomas Monjalon
thomas at monjalon.net
Thu Jan 4 14:15:23 CET 2024
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.
More information about the dev
mailing list