From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 588BD4383B; Fri, 5 Jan 2024 10:03:24 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A888F4067D; Fri, 5 Jan 2024 10:03:08 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 535F6402EF for ; Thu, 4 Jan 2024 13:34:54 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7AEE1C15; Thu, 4 Jan 2024 04:35:39 -0800 (PST) Received: from [192.168.50.86] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B28CC3F64C; Thu, 4 Jan 2024 04:34:52 -0800 (PST) Message-ID: <115ac141-685b-4b60-a338-ae446c2ab966@arm.com> Date: Thu, 4 Jan 2024 12:34:51 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] dts: improve documentation Content-Language: en-GB To: Thomas Monjalon Cc: dev@dpdk.org, Lijuan Tu , =?UTF-8?Q?Juraj_Linke=C5=A1?= , Paul Szczepanek References: <20240103125438.182098-1-Luca.Vizzarro@arm.com> <22136588.EfDdHjke4D@thomas> From: Luca Vizzarro In-Reply-To: <22136588.EfDdHjke4D@thomas> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Mailman-Approved-At: Fri, 05 Jan 2024 10:03:03 +0100 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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. >> + (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. > 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. > 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. >> - [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. Nonetheless, I am keen to update the code as part of this patch to resolve your comments. > 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. Thank you a lot for your review! You have made some good points which open up new potential tasks to add to the pipeline. Best, Luca