Repository navigation
Add lint rule to enforce no rST markup in tests/scripts prose - #20929
dd-apm-ecosystems[bot] wants to merge 4 commits into
Conversation
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 16 Pipeline jobs failed
ℹ️ InfoNo other issues found (see more)🧪 All tests passed Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 0042d21 | Docs | View more details | Give us feedback! |
Codeowners resolved asResolved from the full PR diff against |
Circular import analysis
|
Dependency direction analysis
|
BenchmarksBenchmark execution time: 2026-10-09 22:04:47 Comparing candidate commit 0042d21 in PR branch Found 0 performance improvements and 10 performance regressions! Performance is the same for 588 metrics, 10 unstable metrics, 6 known flaky benchmarks, 18 flaky benchmarks without significant changes.
|
Summary
This PR adds a mechanically-detectable lint rule enforcing an existing documented convention: no reStructuredText (rST) double-backtick markup in non-Sphinx-rendered prose (docstrings, comments) under
tests/orscripts/.(a) Review theme and frequency
Scanning review comments on human-authored PRs updated/opened against
DataDog/dd-trace-pyin the trailing 30 days, this exact theme — reviewers flagging newly addedlike_thisrST markup in test/script docstrings or comments as violating the repo's documented "plain prose for non-rendered code" convention — was observed in at least 12 of the ~101 human-authored PRs that received review comments in the window (e.g. PRs #19273, #19857, #19939, #20526, #20565, #20671, #20688, #20700, #20772, #20773, #20777, #20900; a broader 34 PRs had the same markup flagged across all files, not justtests//scripts/). Several of these were explicitly accepted and fixed in follow-up commits, e.g.:Even a single confirmed, accepted, mechanically-detectable occurrence in the window would be sufficient grounds to act (per the standing automation policy for this task) — the fact that this recurred across a dozen distinct PRs is additional supporting evidence that the pattern is persistent and worth automating away.
(b) Why this is a good automation candidate
text(double backticks, not triple-or-more, to avoid Markdown code fences) with essentially no semantic judgment required.check_no_new_aidev_anchors.py), and only insidetests/andscripts/— the two directories where nothing is Sphinx-rendered (verified: noautomoduledirective underdocs/points at either), so every match there is unambiguously a convention violation, never a legitimate rendered docstring.tests//scripts/are untouched; only new occurrences fail CI.(c) Tooling choice and CI integration
This repo already has an idiomatic precedent for exactly this kind of check:
scripts/check_no_new_aidev_anchors.py, a custom incremental-diff Python script (not a generic linter rule), wired into both GitLab CI (.gitlab-ci.yml→check_no_new_aidev_anchorsjob, which blocks merge) andtests/suitespec.yml(so its own unit tests run in theinternaltest suite). I followed this exact pattern rather than introducing a new tool:scripts/check_no_rst_markup_in_prose.py— same structure as the aidev-anchors checker (merge-base diff, added-lines-only, excluded self/own-test files).tests/internal/test_check_no_rst_markup_in_prose.py— unit tests mirroring the aidev-anchors test file's structure.check_no_rst_markup_in_proseGitLab CI job in.gitlab-ci.yml, immediately aftercheck_no_new_aidev_anchors, with identicalstage/rules/id_tokens— this job runs on every PR (via the GitLab mirror) and blocks merge on failure, the same waycheck_no_new_aidev_anchorsdoes today.scripts/check_no_rst_markup_in_prose.pyto theinternalcomponent'spaths:intests/suitespec.yml, next tocheck_no_new_aidev_anchors.py, so the new checker's own unit tests are included in that CI suite.banditscanning inpyproject.toml, mirroring the existing exclusion forcheck_no_new_aidev_anchors.py.AGENTS.mdunder a new "rST Markup in tests/ and scripts/" section, next to the existing "AIDEV Anchor Comments" section it mirrors.I also attempted to add a GitHub Actions
pull_requestworkflow (mirroringcheck-no-new-aidev-anchors.yml) as an additional PR-only gate, but the automation's GitHub App lacks permission to write.github/workflows/*; the GitLab CI job above is the actual existing merge-blocking mechanism for this repo (GitHub Actions workflows here appear to be supplementary/PR-context-only duplicates of GitLab jobs), so it alone is sufficient.(d) How to verify locally
A clean run prints
OK: no new rST double-backtick markup on added lines under tests/ or scripts/.and exits 0; a violation prints the offending file/line and exits 1.Opened as a draft per the automated code-review-efficiency workflow. Human review/merge required.