Skip to content

Type annotations - #162

Open
JulienPalard wants to merge 10 commits into
sphinx-contrib:mainfrom
JulienPalard:mdk-typing
Open

Type annotations#162
JulienPalard wants to merge 10 commits into
sphinx-contrib:mainfrom
JulienPalard:mdk-typing

Conversation

@JulienPalard

Copy link
Copy Markdown
Collaborator

Spotted some files paths were passed as string instead of Path, while changing that I added a few annotations to disambiguate str vs Path, and did the rest.

@hugovk hugovk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be good to add a type checker to CI, which will probably need some more adjustments, but that can be a follow up.

Comment thread sphinxlint/cli.py
Comment thread sphinxlint/sphinxlint.py Outdated
Comment thread sphinxlint/sphinxlint.py
Comment thread sphinxlint/cli.py Outdated
Comment thread sphinxlint/cli.py Outdated
@JulienPalard

Copy link
Copy Markdown
Collaborator Author

Thank you very much @hugovk for this review!

I'll try to add a checker in the CI in another PR.

@JulienPalard

Copy link
Copy Markdown
Collaborator Author

Hum, Windows...

Comment thread sphinxlint/cli.py
return os.cpu_count() or 1
return max(int(values), 1)

def string_with_no_backslash(user_input):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def string_with_no_backslash(user_input):
def string_with_no_backslash(user_input: str) -> str:

Comment thread sphinxlint/cli.py
if os.path.isfile(path):
if path in ignore_list:
if path.is_file():
if str(path) in ignore_list:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also use is_ignored() here so the path slashes are normalised, similar to the path branch below?

Suggested change
if str(path) in ignore_list:
if is_ignored(path, ignore_list)::

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants