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 AC3BF45713; Thu, 1 Aug 2024 15:03:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 99BA542F0F; Thu, 1 Aug 2024 15:03:12 +0200 (CEST) Received: from mail-ej1-f49.google.com (mail-ej1-f49.google.com [209.85.218.49]) by mails.dpdk.org (Postfix) with ESMTP id 9A09940A73 for ; Thu, 1 Aug 2024 15:03:10 +0200 (CEST) Received: by mail-ej1-f49.google.com with SMTP id a640c23a62f3a-a728f74c23dso912610766b.1 for ; Thu, 01 Aug 2024 06:03:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1722517390; x=1723122190; darn=dpdk.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=MCVsU/QD5hhbrAyhvC+IgLKeMeKkksasvHM+k4Sdg+s=; b=m67h98H8hcwKmvIb05cG4z27jlwRYkTlvTL1KJrCcx+YTX6rm8/0fhaP+W6ZPJ1CyX 8naBJ5wfVouFboD4+AoErG7aPasC2SPMl1Wdmas/db8WQ0cbHDerOW/4Tz3E63UN5ba6 LJj1bLH9Ucb1X2mVVqfVMXpN3j8ZoOadDZxGzM6KCjPB9thlwKNTUtSoJG4fyMOcgACu DSXaCTeuCgZWHsDNr+ZMYurVm083CNDaWNBv+p3w7eTRZm/m2GgOjVWPGK2gzRqj9PKc NJyoiIk6yV5K+S5t1u2M519vjg50CJalwIkCok2oTD3rA2DOBNsJ3PCBo0hBlU/XC1ZH jyHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722517390; x=1723122190; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=MCVsU/QD5hhbrAyhvC+IgLKeMeKkksasvHM+k4Sdg+s=; b=rXLnnGe3xbLqT7LpKB30vQmBrZ97uAND9n4drHL/Q/dcbArcgEquiQcgKHgPI78uvH nzgzSzuxANGFltxJR6IEmWqW6RZfljsGQQqytgkEKGxtncdYGvpWiZeN4ymTZu5LKFyk bp90AwHLg1hK62c7RXehjDm1K/P/cRba9Nh1Ap1qmUBNetKQC3N+TIHBfb2Wb/D2CjRA TztuQB7RJ6kS+fdjOmCDizpsUrfYArk0t+z9z1rCSHtwdg4Q46dYc1g8DRZ12rVhAqJ4 3lfyR/Z1W45GUcjuq/d1ikB7pFA6jpZ3nE963DGcpSQ/UTifP6J1AO+MW5kXvNdHxfEO kYUQ== X-Forwarded-Encrypted: i=1; AJvYcCVH2MeqzlDE2p9ObCY+SdSRihkTUukFsH58AK3P65IaQ1P27AkynCNAAM9WaCNEzxSloC5zA2INFzwxhkw= X-Gm-Message-State: AOJu0YzRjJRTyWg4sMJzLTP3HOW6jsA5TfDoEYJ366Y1MmmjbCOtBR/j y8kr0tlnfXZjVBVsAgPoUHtFqWHgR0kVuXAoJSpsZ29dmBglRjQesuWFeeBgcVw= X-Google-Smtp-Source: AGHT+IHRBJrMj6A7YfX5g7xocRa3918Wtk4M8HitlsNJPJXCrkdySnUxyPOZ9TBj87Xk2YbcJ/FMdw== X-Received: by 2002:a17:906:c10e:b0:a7a:929f:c0cf with SMTP id a640c23a62f3a-a7dc4df95d6mr7731566b.21.1722517389876; Thu, 01 Aug 2024 06:03:09 -0700 (PDT) Received: from [192.168.200.22] ([84.245.121.236]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a7acab5359bsm897688466b.87.2024.08.01.06.03.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 01 Aug 2024 06:03:09 -0700 (PDT) Message-ID: <9cfefccb-0190-46d3-8942-c8c82da38f99@pantheon.tech> Date: Thu, 1 Aug 2024 15:03:07 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v8 5/5] dts: add API doc generation To: Thomas Monjalon Cc: Honnappa.Nagarahalli@arm.com, bruce.richardson@intel.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, npratte@iol.unh.edu, dev@dpdk.org References: <20231115133606.42081-1-juraj.linkes@pantheon.tech> <20240712085724.21065-1-juraj.linkes@pantheon.tech> <20240712085724.21065-6-juraj.linkes@pantheon.tech> <10378083.0AQdONaE2F@thomas> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: <10378083.0AQdONaE2F@thomas> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 30. 7. 2024 15:51, Thomas Monjalon wrote: > 12/07/2024 10:57, Juraj Linkeš: >> The tool used to generate DTS API docs is Sphinx, which is already in >> use in DPDK. The same configuration is used to preserve style with one >> DTS-specific configuration (so that the DPDK docs are unchanged) that >> modifies how the sidebar displays the content. > > What is changed in the sidebar? > These are the two changes: html_theme_options = { 'collapse_navigation': False, 'navigation_depth': -1, } The first allows you to explore the structure without needing to enter any specific section - it puts the + at each section so everything is expandable. The second just means that each section can be fully expanded (there's no limit). > >> --- a/doc/api/doxy-api-index.md >> +++ b/doc/api/doxy-api-index.md >> @@ -244,3 +244,6 @@ The public API headers are grouped by topics: >> [experimental APIs](@ref rte_compat.h), >> [ABI versioning](@ref rte_function_versioning.h), >> [version](@ref rte_version.h) >> + >> +- **tests**: >> + [**DTS**](@dts_api_main_page) > > OK looks good > > >> --- a/doc/api/doxy-api.conf.in >> +++ b/doc/api/doxy-api.conf.in >> @@ -124,6 +124,8 @@ SEARCHENGINE = YES >> SORT_MEMBER_DOCS = NO >> SOURCE_BROWSER = YES >> >> +ALIASES = "dts_api_main_page=@DTS_API_MAIN_PAGE@" > > Why is it needed? > That's the only way to reference it in doxy-api-index.md? > Would be nice to explain in the commit log. > I can add something to the commit log. The questions are answered below, in your other related comment. >> --- a/doc/api/meson.build >> +++ b/doc/api/meson.build >> +# A local reference must be relative to the main index.html page >> +# The path below can't be taken from the DTS meson file as that would >> +# require recursive subdir traversal (doc, dts, then doc again) > > This comment is really obscure. > I guess it is. I just wanted to explain that there's not way to do this without spelling out the path this way. At least I didn't find a way. Should I remove the comment or reword it? >> +cdata.set('DTS_API_MAIN_PAGE', join_paths('..', 'dts', 'html', 'index.html')) > > Oh I think I get it: > - DTS_API_MAIN_PAGE is the Meson variable > - dts_api_main_page is the Doxygen variable > Yes, this is a way to make it work. Maybe there's something else (I'm not that familiar with Doxygen), but from what I can tell, there wasn't a command line option that would set a variable (passing the path form Meson to Doxygen) and nothing else I found worked. Is this solution ok? If we want to explore something else, is there someone with more experience with Doxygen who could help? > >> +# Napoleon enables the Google format of Python doscstrings, used in DTS >> +# Intersphinx allows linking to external projects, such as Python docs, also used in DTS > > Close sentences with a dot, it is easier to read. > Ack. >> +extensions = ['sphinx.ext.napoleon', 'sphinx.ext.intersphinx'] >> + >> +# DTS Python docstring options >> +autodoc_default_options = { >> + 'members': True, >> + 'member-order': 'bysource', >> + 'show-inheritance': True, >> +} >> +autodoc_class_signature = 'separated' >> +autodoc_typehints = 'both' >> +autodoc_typehints_format = 'short' >> +autodoc_typehints_description_target = 'documented' >> +napoleon_numpy_docstring = False >> +napoleon_attr_annotations = True >> +napoleon_preprocess_types = True >> +add_module_names = False >> +toc_object_entries = True >> +toc_object_entries_show_parents = 'hide' >> +intersphinx_mapping = {'python': ('https://docs.python.org/3', None)} >> + >> +dts_root = environ.get('DTS_ROOT') > > Why does it need to be passed as an environment variable? > Isn't it a fixed absolute path? > The path to DTS needs to be passed in some way (and added to sys.path) so that Sphinx knows where the sources are in order to import them. Do you want us to not pass the path, but just hardcode it here? I didn't really think about that, maybe that could work. >> +if dts_root: >> + path.append(dts_root) >> + # DTS Sidebar config >> + html_theme_options = { >> + 'collapse_navigation': False, >> + 'navigation_depth': -1, >> + } > > [...] > >> +To build DTS API docs, install the dependencies with Poetry, then enter its shell: > > I don't plan to use Poetry on my machine. > Can we simply describe the dependencies even if the versions are not specified? > The reason we don't list the dependencies anywhere is that doing it with Poetry is much easier (and a bit safer, as Poetry is going to install tested versions). But I can add references to the two relevant sections of dts/pyproject.toml which contain the dependencies with a note that they can be installed with pip (and I guess that would be another dependency), but at that point it's that not much different than using Poetry. >> + >> +.. code-block:: console >> + >> + poetry install --no-root --with docs >> + poetry shell >> + >> +The documentation is built using the standard DPDK build system. >> +After executing the meson command and entering Poetry's shell, build the documentation with: >> + >> +.. code-block:: console >> + >> + ninja -C build dts-doc > > Don't we rely on the Meson option "enable_docs"? I had a discussion about this with Bruce, but I can't find it anywhere, so here's what I remember: 1. We didn't want to tie the dts api doc build to dpdk doc build because of the dependencies. 2. There's a way to build docs without the enable_docs option (running ninja with the target), which is what we added for dts. This doesn't tie the dts api doc build to the dpdk doc build. 3. We had an "enable_dts_docs" Meson option in the past (to keep it separate from dpdk doc build), but decided to drop it. My memory is hazy on this, but I think it was, again, because of the additional steps needed to bring up the dependency (poetry shell) - at that point, supporting just the ninja build way is sufficient. Bruce may shed more light on this. >> + >> +The output is generated in ``build/doc/api/dts/html``. >> + >> +.. Note:: > > In general the RST expressions are lowercase. > Ack. >> + >> + Make sure to fix any Sphinx warnings when adding or updating docstrings, >> + and also run the ``devtools/dts-check-format.sh`` script and address any issues it finds. > > It looks like something to write in the contributing guide. > I could add it there, where is the right place? In patches.rst, section "Checking the Patches"? > >> +++ b/dts/doc/meson.build >> @@ -0,0 +1,27 @@ >> +# SPDX-License-Identifier: BSD-3-Clause >> +# Copyright(c) 2023 PANTHEON.tech s.r.o. >> + >> +sphinx = find_program('sphinx-build', required: false) >> +sphinx_apidoc = find_program('sphinx-apidoc', required: false) >> + >> +if not sphinx.found() or not sphinx_apidoc.found() > > You should include the option "enable_docs" here. > >> + subdir_done() >> +endif > > >