Repository navigation
ci: add Markdown docstring check - #1393
Aaron ("AJ") Steers (aaronsteers) merged 1 commit into
Conversation
Co-Authored-By: AJ Steers <aj@airbyte.io>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1792118241-docstring-markdown-check' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1792118241-docstring-markdown-check'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
Code coverage overviewLanguages: Python Python / code-coverage/pytest-no-creds
View coverage of the 10 most impacted files.
Python / code-coverage/pytest
View coverage of the 10 most impacted files.
Python / code-coverage/pytest-unit
Line coverage data for the release-candidate/v1 branch is not yet available. View coverage of the 10 most covered files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
30c42ad
into
release-candidate/v1
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| _DEFAULT_PACKAGE = "airbyte" | ||
|
|
||
| # Double backticks, excluding a run of three or more (fence delimiters). | ||
| _DOUBLE_BACKTICK = re.compile(r"(?<!`)``(?!`)") |
There was a problem hiding this comment.
🟡 Code examples falsely fail docstring checks
When a fenced Markdown example contains ``, _DOUBLE_BACKTICK flags it as invalid inline markup. Valid examples then fail CI despite containing no RST formatting.
Learn more
_DOUBLE_BACKTICK matches pairs anywhere in a docstring. Markdown code fences can contain arbitrary source code, including doubled backticks, without using reStructuredText formatting. The checker already permits three-backtick delimiters, but still scans the example inside them.
Example: A docstring with a fenced Python block containing print('``') reports a double-backtick violation and exits 1, although the fenced block is valid Markdown.
Recommended fix: Exclude fenced code blocks before searching for inline RST syntax. Cover both backtick and tilde Markdown fences in tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid edge case, but nothing in the codebase triggers it. The checker passes on release-candidate/v1, so no current docstring has a fenced example containing double backticks. This script is a verbatim copy of the ai-skills checker (only _DEFAULT_PACKAGE and the hint path differ). Skipping fenced blocks should therefore go into ai-skills first and then be re-copied here, so the two copies don't drift. Not opening a follow-up for now. If a real docstring ever hits this, the CI failure will name the file and line.
Overview
👉 TL;DR: Adds a CI check that fails when docstrings use reStructuredText markup instead of Markdown. pdoc renders docstrings as Markdown, so RST shows up literally in the published docs.
Specifically, this adds
scripts/check_docstring_markdown.uv(copied from the ai-skillspython-coding-standardsskill), aPython Docstring Format Checkjob inpython_lint.yml, and apoe lint-docstringstask.Changes
scripts/check_docstring_markdown.uv: the ai-skills checker, verbatim except_DEFAULT_PACKAGE = "airbyte"and the CONTRIBUTING path in the hint. AST-scoped to docstrings only; flags double backticks,:meth:-style roles, and:param:-style field lists..github/workflows/python_lint.yml: newdocstring-markdown-checkjob. It needs onlysetup-uv; the script's shebang pins Python.pyproject.toml: newlint-docstringsPoe task, also appended topoe check.CONTRIBUTING.md: one bullet under## Docstring Stylenaming the job and the local command.airbyte/_util/api_util.pyandairbyte/cli/smoke_test_source/source.py.Design notes: one PR instead of the skill's rule + effects stack
Show/Hide Content
The ai-skills recipe recommends two stacked PRs (rule, then mechanical fixes) because most codebases start with hundreds of violations. RC had only 9, so the fixes ride along and this PR is green on its own.
Test plan
./scripts/check_docstring_markdown.uvandpoe lint-docstringspass on the branch.``x``made the checker exit 1 (fixture not committed).ruff check,ruff format --checkon the edited files;actionlint .github/workflows/python_lint.yml.Link to Devin session: https://app.devin.ai/sessions/4d1ece6885c44c52a07034e025db11e6
Open in Devin Desktop: https://app.devin.ai/desktop/session/4d1ece6885c44c52a07034e025db11e6?variant=devin
Requested by: Aaron ("AJ") Steers (@aaronsteers)
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.