Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] doc: Add Sphinx extension for code samples #62029

Merged
merged 4 commits into from Sep 6, 2023

Conversation

kartben
Copy link
Collaborator

@kartben kartben commented Aug 29, 2023

CI-built doc website: https://builds.zephyrproject.io/zephyr/pr/62029/docs/

Introduction

Finding relevant code samples for a given API can be challenging, and this RFC aims at introducing some Sphinx tooling to help make documentation of code samples more consistent / useful.

Problem description

Unless one is already well-versed into Zephyr (and has a way to actually browse the source tree), finding code samples that are relevant for a particular API can be very challenging.

Proposed change

Add a Sphinx extension that helps formally describe the most critical information for each code samples:

  • its name
  • its description
  • the list of APIs it is a good showcase of
  • (maybe: the board/boards/board regex in case the sample is board-specific?)

Detailed RFC

This PR introduces a new Sphinx extension meant at providing a way to document code samples in a more structured way, and to have a way for users to discover relevant samples when browsing a given API documentation page.
As a proof of concept, a couple basic samples have been ported over. Should more or less fix #60647.

Example usage for the zephyr:code-sample:: directive allowing to describe a sample:

.. zephyr:code-sample:: synchronization_sample
   :name: Synchronization Sample
   :doxygen-groups: thread_apis semaphore_apis

   A simple application that demonstrates basic kernel synchronization
   primitives.

Example usage for the zephyr:code-sample role:

You can learn more about this in :zephyr:code-sample:`synchronization_sample` 

Some of the things that this enables (not everything implemented in the PR, but might be in the future based on feedback / interest:

  • Each sample have a clearly defined name and short description
  • No more need for declaring an explicit target at the beginning of each sample doc file since code-sample role now allows to link to a sample's
  • Make "code samples" a first class citizen in Zephyr, ex. they can be treated in a special way in search results, etc.
  • Ability to indicate Zephyr APIs (in the form of Doxygen groups) that the code sample is showcasing, ex. for the synchronization sample that has semaphore_apis listed as one of the doxygen-groups, Semaphore API now looks like this (admonition is collapsed by default -- don't mind the buggy style if you test it live here, I'll get this fixed):
image
  • Ability to create a better looking table of contents for the code samples, i.e all samples have a meaningful short description, are sorted alphabetically,...
  • We could potentially "tag" samples with keywords/categories to make the samples catalog something that people can easily navigate by having a UI for showing all samples that e.g. showcase "security". We already have samples grouped in logical folders in the tree, but we could maybe benefit from more flexibility (ex. such or such "networking" sample might also be very relevant for "crypto", or "sensors", or whatever).
  • Board-specific samples could indicate which board (.... or boardS, using a regex?) they're for, so that the documentation page for a board can show the list of related samples. Probably want to have a .. zephyr:board directive at some point?

Note: I went for Doxygen groups as the type of "unit" that is relevant to attach to a code sample, but we could think of different/additional ones too - ex. header files, C functions (i.e. "checkout this one sample cause it has everything you need to understand how to use this particular function"), ...

Concerns and Unresolved Questions

  • Need to make sure this doesn't impact documentation build time
  • Need to perform tree-wide migration of existing samples when/if extension lands

Alternatives

  • This could be done the other way around, i.e. we add some kind of Doxygen extension to flag relevant code samples directly from the API headers (or anywhere really)
  • Status quo, with an ever growing list of 450+ code samples

@kartben kartben force-pushed the sphinx_code_samples_ext branch 25 times, most recently from 76177ff to 59b921d Compare August 30, 2023 15:58
@kartben kartben marked this pull request as ready for review August 30, 2023 17:01
@kartben kartben added area: Documentation RFC Request For Comments: want input from the community labels Aug 30, 2023
@kartben
Copy link
Collaborator Author

kartben commented Sep 5, 2023

Latest updates address the following issues/comments raised by folks:

  • Extension is now zephyr.domain (and it's likely that other roles/directives such as zephyr-file etc. should move there over time, like @gmarull pointed out). I used the opportunity to clean up the way code samples are stored in the environment, i.e they now are stored as domain-specific data.
  • Renamed doxygen-groups to relevant-api
  • Updated sample.tmpl file to invite people to use the new extension. Added wording to try and aim for consistency in the way the short description of samples should be phrased, i.e. It is recommended to word this as if you were completing the sentence "This code sample shows how to ...". I am hoping to eventually use this short description in a future revamp of the samples index page, and it would be nice to have consistency in how samples are described
  • Documented the condition variables sample with the new method as it is listed here as an example of a well documented sample (but it might not be the best TBH)
  • Fixed the CSS style for the collapsible admonition
  • The list of related code samples is sorted alphabetically

@kartben kartben force-pushed the sphinx_code_samples_ext branch 4 times, most recently from 9fd99be to 4edf3bf Compare September 5, 2023 19:17
@kartben
Copy link
Collaborator Author

kartben commented Sep 5, 2023

  • renamed doxygen-groups to relevant-api

FWIW I am very tempted to also add a :relevant-kconfig: option 👀

@kartben
Copy link
Collaborator Author

kartben commented Sep 6, 2023

@carlescufi @cfriedt @cfriedt I think this is ready for another review. Please see #62029 (comment) for main changes.

I didn't want to do it initially to keep the initial RFC changes small as a proof of concept, but if people think it's a good idea I could update the third commit of the series so that it effectively migrates all the basic/ samples to the new directive. See the full changes in 391204f.

This would allow to remove the following code that's currently here only to keep backwards compatible with "old" link targets.

# TODO remove once all :ref:`sample-xyz` have migrated to :zephyr:code-sample:`xyz`
# as this is the recommended way to reference code samples going forward.
self.env.app.env.domaindata["std"]["labels"][node['id']] = self.env.docname, node['id'], node['name']
self.env.app.env.domaindata["std"]["anonlabels"][node['id']] = self.env.docname, node['id']

Thoughts?

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall lgtm, thanks for this addition. It would also be nice to render the information in a reversed way, ie, in samples, list the relevant APIs used in the sample. So when I open eg blinky, I'll get a link to the GPIO API.

doc/_extensions/zephyr/domain.py Outdated Show resolved Hide resolved
doc/_extensions/zephyr/domain.py Outdated Show resolved Hide resolved
doc/_extensions/zephyr/domain.py Outdated Show resolved Hide resolved
samples/kernel/condition_variables/condvar/README.rst Outdated Show resolved Hide resolved
This adds a new Sphinx extension for both a code-sample directive and role.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Update the sample.tmpl file so that it encourages people to use the new
:zephyr:code-sample: directive.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Use the new code-sample directive and roles in a few places
to demonstrate how it works.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Use sphinx-toggle to make "Related code samples" collapsible.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
@kartben
Copy link
Collaborator Author

kartben commented Sep 6, 2023

overall lgtm, thanks for this addition. It would also be nice to render the information in a reversed way, ie, in samples, list the relevant APIs used in the sample. So when I open eg blinky, I'll get a link to the GPIO API.

Ya I will keep this for a later update, I think. I thought about adding it but what kept me from doing it TBH was that for some reason Breathe does not seem to capture the human-readable name of doxygen groups (but I could probably put together some kind of hack I'm sure). So it would be looking rather ugly IMO to have the references render as "Related API: gpio_interfaces" where we could instead have "Related API: GPIO Driver APIs"

@gmarull
Copy link
Member

gmarull commented Sep 6, 2023

overall lgtm, thanks for this addition. It would also be nice to render the information in a reversed way, ie, in samples, list the relevant APIs used in the sample. So when I open eg blinky, I'll get a link to the GPIO API.

Ya I will keep this for a later update, I think. I thought about adding it but what kept me from doing it TBH was that for some reason Breathe does not seem to capture the human-readable name of doxygen groups (but I could probably put together some kind of hack I'm sure). So it would be looking rather ugly IMO to have the references render as "Related API: gpio_interfaces" where we could instead have "Related API: GPIO Driver APIs"

We should also be cautious about building stuff on top of breathe, it's not well maintained.

@carlescufi carlescufi merged commit b2977b7 into zephyrproject-rtos:main Sep 6, 2023
16 checks passed
@kartben kartben deleted the sphinx_code_samples_ext branch September 15, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth area: Documentation Infrastructure area: Documentation area: DX Developer and User Experience area: hash utils Hash Functions and Hash Maps (Hash Tables) area: Kernel platform: nRF Nordic nRFx RFC Request For Comments: want input from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

documentation: Consider adding reference to relevant code samples in the Doxygen documentation
7 participants