diff --git a/docs/source/contributing/conventions/documentation.rst b/docs/source/contributing/conventions/documentation.rst new file mode 100644 index 000000000..dc3c65271 --- /dev/null +++ b/docs/source/contributing/conventions/documentation.rst @@ -0,0 +1,297 @@ +.. _documentation: + +Writing the Documentation +------------------------- + +The pages before this one rule on library code and on running the repository. This +one rules on the documentation itself -- how a page is titled, when a diagram beats a +paragraph, and what a sentence on it is allowed to claim. It governs the +reStructuredText under :file:`docs/source/` and the :mod:`pcapkit` docstrings the API +reference renders from, since the owner named both when settling the first of these. + +Nearly all of it was settled on +`#719 `__, the prose sweep, whose +thread was the only place most of it lived. As on :ref:`process`, every ruling here is +**paraphrased rather than quoted**, on the owner's standing instruction there; the +issue named beside a rule is where the original wording is. + +Heading Case and Shape +~~~~~~~~~~~~~~~~~~~~~~ + +**Title Case, and short.** Sentence case belongs only to a heading that genuinely is a +sentence -- a how-to question is the example the owner gave -- and that was ruled rare: +a sentence should generally not be used as a title at all. Settled on #719. + +Title Case here is the conventional kind rather than every-word capitalisation. The +short function words ``a``, ``an``, ``the``, ``and``, ``or``, ``of``, ``in``, ``for`` +and ``to`` stay lowercase unless they lead, which was the reading put to the owner on +#719 and left standing. It is also what the tree does: *The* ``all`` *Extra* and +*Issue and Pull Request Labels* on :ref:`process` are both in it. + +**The casing is the easy half.** A heading can be in Title Case already and still +break the rule by being a clause rather than a title. The sweep of +:file:`docs/source/contributing/` under this ruling found, of 62 headings, 24 already +right, 16 needing only re-casing, and **22 to be rewritten**. *What a Failed Lookup +Raises* was correctly cased and is now *Failed-Lookup Exceptions*. + +The line that sweep drew, and the one to keep drawing: a **finite verb** makes +a heading a sentence and earns a rewrite, while a gerund or infinitive phrase is a +noun phrase and needs only re-casing -- so *Running the tests* became *Running the +Tests* rather than being reworded. + +.. note:: + + A heading also has to describe the section beneath it, and a rewrite is the moment + to check that. Two of that sweep's own new titles were renamed again in review for + failing it. *One Approval, Whole Release* became *The Release Pipeline* because the + section opens on a Mermaid flowchart while the approval prose it had been named for + sat well below, leaving the page's most navigable artefact unfindable from the table + of contents. + +Renaming a Heading +~~~~~~~~~~~~~~~~~~ + +Two mechanical traps, both of which bit on that sweep rather than being hypothetical. + +**Re-measure the underline against the new text.** A case flip usually keeps the +length; a rewrite almost never does, and docutils reports an underline shorter than +its heading in a build that does not fail on warnings. The sweep re-checked all 62 and +found none short, which is the standard to hold. + +**A heading is a link target, so a rename is a repository-wide sweep** rather than a +:file:`docs/source/` one. In scope: ``.rst`` prose, ``:ref:`` and ``:doc:`` link text, +toctree entries, implicit ``` `Text`_ ``` references, **and** :mod:`pcapkit` +docstrings and the files under :file:`tests/`. The first pass reported one +surviving stale reference; review found **five**, spread across +:file:`pcapkit/corekit/sentinels.py`, +:file:`tests/corekit/test_sentinel_exports_unit.py` and +:file:`tests/project/test_conventions_doc_claims.py`. The first of those is a shipped +module docstring that renders into the API reference, so a reader following it +searches the page for a string no longer on it. + +.. warning:: + + **Derive the list of old headings from the pre-change file, not from the diff.** A + diff-derived list silently drops any heading whose underline is not adjacent in the + hunk, which is why the second pass undercounted as well as the first. What + worked was a whitespace-flattened search for every pre-change heading string across + every tracked file. + +Nothing in CI catches a reference a rename left behind. :file:`docs/source/conf.py` +sets no ``nitpicky`` and :file:`docs/Makefile` leaves ``SPHINXOPTS`` empty, so the +build runs with neither ``-n`` nor ``-W``: a dead reference renders as the plain text +it used to be, and the build still succeeds. ``#934`` found sixteen of them at once +that way. + +One thing a rename breaks that no tool checks at all is the prose around it. Turning a +singular heading plural cost :ref:`registry-protocol` the antecedent of a following +*"it"*, caught in the same review. + +Mermaid for Flows +~~~~~~~~~~~~~~~~~ + +Where the subject is a flow, prefer a Mermaid graph to the paragraph or the ASCII +diagram that would otherwise carry it -- a graph is read faster than its own +description. The owner ruled this on #719, asking for it where it is necessary and +helpful, which bounds it in three directions: + +* **A short sequence does not earn a graph.** Two steps read perfectly well as a + sentence, and a diagram of them costs a reader a context switch for nothing. +* **A rationale stays prose.** A diagram carries structure and sequence; it cannot + carry *why* a choice was made, and that reasoning is what #719 protects rather than + compresses. +* **Do not redraw a graph another page already has.** The owner's condition when + approving the navigation work on #719 was that nothing duplicate information already + shown, and a second copy of a flow is exactly that. + +The style model is the set already in the tree, every one of which builds. The sweep +excludes this page, which writes the directive name three times in its own prose and +would otherwise inflate both counts: + +.. code-block:: shell + + grep -rn 'mermaid::' docs/source --include='*.rst' \ + --exclude='documentation.rst' | wc -l # 12 directives + grep -rl 'mermaid::' docs/source --include='*.rst' \ + --exclude='documentation.rst' | wc -l # on 11 pages + +All twelve are a bare ``.. mermaid::`` carrying no directive options, and all twelve +are a ``flowchart``: ``TD`` where the subject is a sequence or a decision, ``LR`` for a +type hierarchy. **Quote a node label whose text Mermaid would otherwise try to parse**, +and leave a bare identifier bare. That is what the exemplars do with *node* labels, and +it falls close to the ``TD``/``LR`` line without following it: every node label in the +six ``TD`` graphs is quoted, since every one of them is prose; the ``LR`` graphs quote +only where the text forces it, which today is ``h1["HTTP/1.*"]`` and ``h2["HTTP/2"]`` in +:file:`docs/source/pcapkit/protocols/index.rst` and nowhere else. Everything else there +uses the bare ``A{{Meta}}``, ``B(Base)``, ``D([user customisation ...])`` and +``subgraph name [Title]`` forms, and spends its quotes on ``click`` targets instead. +**Edge labels follow no rule at all.** The exemplars quote some and leave others bare, +including ``|workflow_run: completed|`` and ``|02:00|``, which the reason above would +have quoted and which Mermaid accepts anyway -- so match the graph being edited rather +than this paragraph. Inside a quoted label, ``
`` breaks the text across lines and a +literal ``<`` or ``>`` is written ``<`` or ``>``. +:doc:`The release pipeline ` is the ``TD`` example and +:doc:`the field hierarchy ` the ``LR`` one. + +Toctree Captions +~~~~~~~~~~~~~~~~ + +**A caption renders wherever its own toctree renders, which is not the same place for +every toctree.** Measured against a built tree rather than assumed: + +* A toctree in the **root** document renders into the global sidebar, so its caption + is visible from every page in the build. +* A **nested** toctree renders only in its own page's body, so its caption is visible + on that page alone. +* ``:hidden:`` suppresses the body rendering. On a root toctree the sidebar copy + survives and the caption is still everywhere; on a nested one nothing is left, and + the caption renders nowhere. + +That is why most of the per-package toctrees are deliberately bare -- a caption on a +package index would be invisible on every page but one. The three in +:file:`docs/source/index.rst` carry captions, and ``Subpackages`` in +:file:`docs/source/pcapkit/index.rst` is the single nested one. A built tree says so -- +excluding this page, which names both captions in its own prose and would otherwise +count itself: + +.. code-block:: shell + + grep -rl 'API Reference' --include='*.html' --exclude='documentation.html' \ + docs/build/html | wc -l # every other page + grep -rl 'Subpackages' --include='*.html' --exclude='documentation.html' \ + docs/build/html | wc -l # exactly one + +That is why those root toctrees are ``:hidden:``. Without it each of the three +captions rendered twice on the root page -- once inline in the body, once in the +sidebar -- which is the duplication the owner ruled out on #719. + +.. note:: + + For the same reason, do not add an inline ``.. contents::`` to a page. The ``furo`` + theme already renders a sticky page-local table of contents on every page, so an + inline directive is a second copy of it; the one in :file:`docs/source/index.rst` + has been commented out since 2023 and should stay that way. + + One setting there was decided by measurement rather than by preference: + ``toc_object_entries`` carries, in a comment beside it, the figures that settled it, + so that nobody reopens the question blind. That is worth copying the + next time a setting is chosen that way, but it is a single precedent and not yet a + rule the owner has ruled on. + +Paraphrasing a Ruling +~~~~~~~~~~~~~~~~~~~~~ + +Write a ruling down in your own words. **Do not quote the owner verbatim** -- a +standing instruction on #719, and the one every page in this directory follows. +``#949`` went back over the five pages that then existed and replaced their quoted +rulings with paraphrase. + +What a quotation costs is not style. A quoted sentence is pinned to the moment it was +said, so it cannot be corrected when the thing it describes moves; and a test that +asserts on the quotation fails when the wording is tidied rather than when the claim +stops being true. That change rewrote those tests alongside the prose, so that each +derives its claim from the tree instead. :ref:`process` and :ref:`mint-criterion` show +the form: say what was ruled, name where it was ruled, and leave the wording there. + +**Name the issue, not the pull request.** A pull request is a point in time: it +describes what was true on the day it merged, and the next change past it can make the +citation wrong without touching it. The issue is the durable half -- where the ruling +was asked for and given -- and it survives the work that implemented it. So cite the +issue a rule was settled on, and describe a change by what it did rather than by its +number. Ruled on #719, and the reason every citation on this page is an issue. + +Accuracy +~~~~~~~~ + +**Verify a claim against the code it describes, never against another document.** Where +prose and code disagree the code wins and the prose is what gets fixed -- #719's own +charter -- and a docstring outliving the thing it described is a demonstrated failure +mode here rather than a hypothetical one. + +**Re-derive a count; do not copy one.** Better still, write down the command that +produces it, as :ref:`process` does, so the figure can be rechecked rather than +trusted. A keyword search is not a sweep: the method the changelog review settled takes +every cited repository path in each of the forms it gets written in -- slash path, +dotted module, bare filename, and the elided spellings -- intersects them with the +diff since the merge base, and re-measures the intersection. A tense-keyword grep +misses a claim phrased as a fraction of a total. + +**Treat** ``every``, ``all``, ``each`` **and** ``none`` **as a claim about members, and +check the members one at a time.** Several of #719's findings were of exactly that +shape: + +* A sweep asserted that every ``.. module::`` target in the documentation resolved. + One did not: :file:`docs/source/pcapkit/protocols/link/rarp.rst` declared + ``pcapkit.protocols.data.link.rarp``, which has never existed, because RARP and + DRARP reuse ARP's data class. Fixed in ``68fbccd90``. +* ``#911``'s ruling -- export the sentinel objects and leave their types out -- was + read as describing all three modules that then held a sentinel. One ran the other + way: :mod:`pcapkit.corekit.fields.field` exported neither, so applying the rule + there meant *adding* a name rather than removing one. + +**Some numbers on these pages are pinned, and knowing which is part of writing one.** +:file:`tests/project/test_conventions_doc_claims.py` re-derives figures for the three +pages that have a figure test of their own -- ``extension-header-subclassing``, +``process`` and ``registry-protocol``, the last most heavily of all, down to both halves +of an *N of M* -- so a stale count there goes red instead of quiet. Every other figure, +this page's included, is pinned by nothing: the entry it adds to that module's +``ANCHORS`` fixes the page's existence, anchor and toctree position, not its arithmetic. +Re-derive before trusting, and do not read a number as load-bearing just because it is +written down. The same module checks something a reader cannot see and Sphinx cannot +warn about: in a ``` `#NNN <.../NNN>`__ ``` link the displayed number has to match the +number in its own URL. A review round once corrupted roughly thirty of them across two +pages and nothing caught it, because both halves were well-formed. + +Resolvable Targets +~~~~~~~~~~~~~~~~~~ + +**A** ``.. module::`` **target must name a file on disk.** A dangling one is worse than +no directive at all: it registers a module-index entry for a module that does not +exist, and gives cross-references a target that resolves to nothing. The sweep that +settled this on #719 found exactly one, and repeating it is cheap: + +.. code-block:: shell + + grep -rhE '^\.\. +(py:)?(module|currentmodule):: ' docs/source --include='*.rst' \ + --exclude='documentation.rst' | + awk '{print $NF}' | sort -u | tr '.' '/' | + while read -r path; do + [ -f "$path.py" ] || [ -f "$path/__init__.py" ] || echo "dangling: $path" + done + +**Every importable shipped module is documented, with one deliberate exception.** The 33 +modules under :file:`pcapkit/protocols/*/NotImplemented/` are the set with no entry: +placeholder sources for dissectors nobody has written yet, carrying no class a reader +could use. The exception is a choice rather than a consequence of packaging, which is +worth stating because the opposite is easy to assume -- measured on ``1.5.0b8``, all 33 +are in the source distribution *and* in the wheel, and each imports as a +namespace-package submodule. What they are not is a *package*: none of the four +directories carries an :file:`__init__.py`, so ``find_packages`` returns 73 here and +omits all of them. + +When something is removed, its documentation entry goes with it -- carrying a +deprecation note where a user could have depended on the thing, and deleted outright +where it never shipped. The ``rarp`` entry above needed no note for that second reason. +Asked and ruled on #719. + +Format and Mechanics +~~~~~~~~~~~~~~~~~~~~ + +* **reStructuredText under** :file:`docs/source/`, **Markdown outside it.** The owner + ruled this on #719, correcting a blanket *always* ``.rst`` that had been in + circulation until then: the Sphinx documentation is reST, and the other documents -- + the READMEs included -- are Markdown where that applies. ``CONTRIBUTING.md``'s own + *Documentation* section records the same split. One trap arrived with the ruling: + :file:`MANIFEST.in` reaches the two READMEs under :file:`examples/` through + ``global-include *.rst`` and has no ``*.md`` equivalent, so renaming one without + adding coverage drops it from every source distribution. +* **Prose names the real defining module, not a re-export.** One strand of the + consistency sweep on :doc:`the roadmap `. The sentinels are the + worked example: all four are defined in :mod:`pcapkit.corekit.sentinels` and + re-exported by the modules that use them, and :ref:`sentinel-convention`'s table + names the defining module rather than any of the re-exports. +* **A verbatim upstream port is exempt from these conventions.** + :ref:`sentinel-convention` already carries the case that settled it, the + ``cached_property`` backport in :file:`pcapkit/utilities/compat.py`: the value of a + vendored copy is that it can still be diffed against upstream, and a house-style + rewrite destroys that in exchange for nothing a reader ever sees. diff --git a/docs/source/contributing/conventions/index.rst b/docs/source/contributing/conventions/index.rst index ba087def9..79448aec0 100644 --- a/docs/source/contributing/conventions/index.rst +++ b/docs/source/contributing/conventions/index.rst @@ -11,8 +11,10 @@ House Conventions :ref:`mint-criterion` and :ref:`registry-protocol` govern :mod:`pcapkit.const`, which is where the settled questions have mostly arisen. :ref:`sentinel-convention` governs :mod:`pcapkit.corekit`, - :ref:`extension-header-subclassing` a protocol class hierarchy, and - :ref:`process` the repository rather than any of its code. + :ref:`extension-header-subclassing` a protocol class hierarchy, + :ref:`process` the repository rather than any of its code, and + :ref:`documentation` the prose itself -- on these pages and in the API + reference -- rather than any code at all. **A ruling that stays in its thread is a ruling that gets rediscovered.** So when a question is answered in a way the code cannot express on its own -- a @@ -30,3 +32,4 @@ House Conventions registry-protocol extension-header-subclassing process + documentation diff --git a/tests/project/test_conventions_doc_claims.py b/tests/project/test_conventions_doc_claims.py index 28e542aa9..2fc6385ab 100644 --- a/tests/project/test_conventions_doc_claims.py +++ b/tests/project/test_conventions_doc_claims.py @@ -102,7 +102,10 @@ #: ``process`` came last, on the owner's ruling that the three settled *process* #: rulings -- the ``all`` extra, what a changelog entry is, and what the labels #: mean -- get a page of their own rather than being wedged onto a code-convention -#: page. Its anchor is the bare file stem like the other four, which is what +#: page. ``documentation`` came after it, carrying the rulings GitHub issue #719 +#: settled about the prose itself -- heading case, when a Mermaid graph beats a +#: paragraph, and what a sentence on these pages may claim -- which had until then +#: lived only in that thread. Every anchor is the bare file stem, which is what #: :data:`PAGES` below depends on. ANCHORS = ( 'mint-criterion', @@ -110,6 +113,7 @@ 'registry-protocol', 'extension-header-subclassing', 'process', + 'documentation', ) #: Each anchor's own file, one-to-one since the split -- there is no longer a single