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.

Sign up for GitHub

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

Add Sphinx Lint to pre-commit #1786

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add Sphinx Lint to pre-commit #1786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
df7ef72
Bump GitHub Actions
hugovk Jan 14, 2025
6339a91
Add Spinx Lint and extra linters to pre-commit
hugovk Jan 14, 2025
e2a552e
Apply requirements-txt-fixer
hugovk Jan 14, 2025
08107d5
Fix tab to spaces, found by Sphinx Lint
hugovk Jan 14, 2025
eb1cf39
Remove duplicate
hugovk Jan 14, 2025
983d39f
Move fast meta hooks to the beginning
hugovk Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test-translations.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ jobs:
run: python -m pip install --upgrade nox virtualenv sphinx-lint

- name: Set Sphinx problem matcher
uses: sphinx-doc/github-problem-matcher@v1.0
uses: sphinx-doc/github-problem-matcher@v1.1

- name: Build translated docs in ${{ matrix.language }}
run: nox -s build -- -q -D language=${{ matrix.language }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ jobs:
- linkcheck

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: "3.11"
cache: 'pip'
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/translation.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ jobs:

steps:
- name: Grab the repo src
uses: actions/checkout@v3
uses: actions/checkout@v4
with:
fetch-depth: 0 # To reach the common commit
- name: Set up git user as [bot]
# Refs:
# * https://github.community/t/github-actions-bot-email-address/17204/6
# * https://github.com/actions/checkout/issues/13#issuecomment-724415212
uses: fregante/setup-git-user@v1.1.0
uses: fregante/setup-git-user@v2.0.2

- name: Switch to the translation source branch
run: |
Expand All @@ -51,7 +51,7 @@ jobs:
git merge '${{ github.event.repository.default_branch }}'

- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: >-
3.10
Expand Down
15 changes: 13 additions & 2 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,22 @@ ci:
autoupdate_schedule: quarterly

repos:
- repo: meta
hooks:
- id: check-hooks-apply
- id: check-useless-excludes

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: check-added-large-files
- id: check-case-conflict
- id: check-merge-conflict
- id: check-symlinks
Copy link
Member

Choose a reason for hiding this comment

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

What's the justification here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flagged by the check-hooks-apply hook. Shall I remove check-hooks-apply and re-add check-symlinks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flagged by the check-hooks-apply hook:

Check hooks apply to the repository......................................Failed
- hook id: check-hooks-apply
- exit code: 1

check-symlinks does not apply to this repository

Shall I remove check-hooks-apply and re-add check-symlinks?

Copy link
Member

Choose a reason for hiding this comment

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

Honesty, not sure. I was just trying to understand how different bits of the PR might be related to the declared goal and each other...

Copy link
Member

Choose a reason for hiding this comment

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

Ned ended up not using this check: #1786 (comment). Perhaps, we shouldn't either.

- id: check-json
Copy link
Member

Choose a reason for hiding this comment

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

Could you add explanation somewhere why one check replaces the other instead of being additional?

Copy link

Choose a reason for hiding this comment

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

I'll take a guess based on what I saw: check-hooks-apply complains that check-symlinks doesn't apply if the repo has no symlinks.

Copy link
Member

Choose a reason for hiding this comment

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

Oh.. I wish that was more apparent.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it... It's rather suboptimal: if we ever start having symlinks, they won't be checked after this patch because nobody will remember to re-add it.

Copy link

Choose a reason for hiding this comment

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

I agree. But perhaps it means we don't want check-hooks-apply.

Copy link

Choose a reason for hiding this comment

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

After trying it in coveragepy, I removed check-hooks-apply.

- id: check-yaml
- id: end-of-file-fixer
- id: mixed-line-ending
- id: requirements-txt-fixer
hugovk marked this conversation as resolved.
Show resolved Hide resolved
- id: trailing-whitespace

- repo: https://github.com/codespell-project/codespell
Expand All @@ -37,7 +43,12 @@ repos:
- id: rst-inline-touching-normal

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.7.1
rev: v0.9.1
hooks:
- id: ruff
- id: ruff-format

- repo: https://github.com/sphinx-contrib/sphinx-lint
rev: v1.0.0
hooks:
- id: sphinx-lint
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
furo==2024.8.6
sphinx==7.2.6
sphinx-autobuild==2021.3.14
sphinx-inline-tabs==2023.4.21
sphinx-copybutton==0.5.2
sphinx-inline-tabs==2023.4.21
sphinx-toolbox==3.5.0
2 changes: 1 addition & 1 deletion source/glossary.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ Glossary
<https://bitbucket.org/pypa>`_, and discuss issues on the
`distutils-sig mailing list
<https://mail.python.org/mailman3/lists/distutils-sig.python.org/>`_
and `the Python Discourse forum <https://discuss.python.org/c/packaging>`__.
and `the Python Discourse forum <https://discuss.python.org/c/packaging>`__.


Python Package Index (PyPI)
Expand Down
24 changes: 12 additions & 12 deletions source/guides/creating-command-line-tools.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ so initizalize the command-line interface here:

.. code-block:: python

if __name__ == "__main__":
from greetings.cli import app
app()
if __name__ == "__main__":
from greetings.cli import app
app()

.. note::

Expand All @@ -123,8 +123,8 @@ For the project to be recognised as a command-line tool, additionally a ``consol

.. code-block:: toml

[project.scripts]
greet = "greetings.cli:app"
[project.scripts]
greet = "greetings.cli:app"

Now, the project's source tree is ready to be transformed into a :term:`distribution package <Distribution Package>`,
which makes it installable.
Expand All @@ -145,12 +145,12 @@ Let's test it:

.. code-block:: console

$ greet --knight Lancelot
Greetings, dear Sir Lancelot!
$ greet --gender feminine Parks
Greetings, dear Ms. Parks!
$ greet --gender masculine
Greetings, dear Mr. what's-his-name!
$ greet --knight Lancelot
Greetings, dear Sir Lancelot!
$ greet --gender feminine Parks
Greetings, dear Ms. Parks!
$ greet --gender masculine
Greetings, dear Mr. what's-his-name!

Since this example uses ``typer``, you could now also get an overview of the program's usage by calling it with
the ``--help`` option, or configure completions via the ``--install-completion`` option.
Expand All @@ -160,7 +160,7 @@ To just run the program without installing it permanently, use ``pipx run``, whi

.. code-block:: console

$ pipx run --spec . greet --knight
$ pipx run --spec . greet --knight

This syntax is a bit unpractical, however; as the name of the entry point we defined above does not match the package name,
we need to state explicitly which executable script to run (even though there is only on in existence).
Expand Down
Loading