-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
doc: sphinx-lint: fix bad usage of "default role" (single backticks) #78266
doc: sphinx-lint: fix bad usage of "default role" (single backticks) #78266
Conversation
df7c4b3
to
d426d9c
Compare
d426d9c
to
dcee786
Compare
@@ -137,11 +137,11 @@ Programmable I/O (PIO) | |||
The RP2040 SoC comes with two PIO periherals. These are two simple | |||
co-processors that are designed for I/O operations. The PIOs run | |||
a custom instruction set, generated from a custom assembly language. | |||
PIO programs are assembled using `pioasm`, a tool provided by Raspberry Pi. | |||
PIO programs are assembled using :command:`pioasm`, a tool provided by Raspberry Pi. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is pioasm
marks as a :command:
when west flash
(or just west
) is not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for paying attention and pointing my inconsistencies :)
https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#role-command
We actually use it in some places across the tree, but definitely not consistently -- e.g. https://docs.zephyrproject.org/latest/develop/debug/index.html#running-qemu-via-west
I would say either way is fine and providing a single recommendation going forward should be a follow-up PR updating the doc guidelines (which I have recently started doing in a branch)
@@ -82,8 +82,8 @@ To upload the bitstream again you need to reset the FPGA: | |||
FPGA: resetting FPGA | |||
|
|||
You can also use your own bitstream. | |||
To load a bitstream into device memory, use `devmem load` command. | |||
It is important to use the -e option when sending a bitstream via `xxd`: | |||
To load a bitstream into device memory, use ``devmem load`` command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto with command
@@ -19,7 +19,7 @@ sysbuild. | |||
This is achieved with a sysbuild specific Kconfig configuration, | |||
:file:`sysbuild.conf`. | |||
|
|||
The `SB_CONFIG_BOOTLOADER_MCUBOOT=y` setting in the sysbuild Kconfig file | |||
The ``SB_CONFIG_BOOTLOADER_MCUBOOT=y`` setting in the sysbuild Kconfig file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not considered a Kconfig option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to admit I have no idea how sysbuild kconfigs work. The idea behind :kconfig:option:
is that it creates a cross-reference to the Kconfig listing. But in this case SB_CONFIG_BOOTLOADER_MCUBOOT is not listed there: https://docs.zephyrproject.org/latest/kconfig.html#SB_CONFIG_BOOTLOADER_MCUBOOT
We could still use the role to convey the idea that it is a kconfig option (just like we could also systematically use the role even for those local CONFIG_SAMPLE_XYZ
option), it would just not link to anything (but still render as a literal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sysbuild Kconfig options are not part of the normal Kconfig output, there is no generation for them, would be good if one day there was @tejlmand
dcee786
to
6904d68
Compare
6904d68
to
d90f22c
Compare
Addressed merge conflict in |
Fixes bad usage of single backticks in lieu of double backticks for rendering inline literals, or simple '*' for italics. When appropriate, a better construct than double backticks has been selected (ex. :file:, :kconfig:option:, :c:func:, ...), or proper :ref: have been used if the original intention was to have a link. Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Commit 2048608 (PR zephyrproject-rtos#41061) dropped support for "any" as the default role, so this should be dropped from the documentation guidelines as well. Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
trivial typo fix where single backticks should be used instead of double for the sphinx role kconfig:option Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
d90f22c
to
defe810
Compare
Had to rebase due to a new annoying merge conflict were I literally had nothing to resolve when rebasing. It just looks like Github was having a hard time with keeping track of files subject to a recent |
This PR removes the incorrect use of single backticks across all documentation files. We haven't had support for Sphinx's "any" default role for a long time (PR #41061) so any remaining single backticks were either due to people wanting "inline literal" formatting (i.e. double backticks), or italics (single
*
).When appropriate, a better construct than double backticks has been selected (ex.
:file:
,:kconfig:option:
,:c:func:
, ...) or proper:ref:
has been used if the original intention was to have a link.The actual fix is in third commit.
The first two commits contain other minor lint fixes.
Last commit drops the section in the documentation guidelines that was still referring to the default any role / single backticks.
Next step after this is merged will be to introduce a Sphinx linter (#78292), as there will basically be no lint errors left anymore 🥳