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.

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

Type hints: toolbar/middleware #1848

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

Conversation

leandrodesouzadev
Copy link
Contributor

Description

This PR adds type hints to the middleware and toolbar modules

Fixes #1705

Checklist:

  • I have added the relevant tests for this change.
  • I have added an item to the Pending section of docs/changes.rst.

@leandrodesouzadev
Copy link
Contributor Author

I haven't touched on the changes.rst file. I wasn't quite sure if it's required for this type of change.

@leandrodesouzadev
Copy link
Contributor Author

I'm not quite sure how to resolve this lint error. Can anyone give me a hint?

@matthiask
Copy link
Member

@leandrodesouzadev The automatically generated documentation for panels (https://django-debug-toolbar.readthedocs.io/en/latest/panels.html#debug_toolbar.panels.Panel) will contain a reference to your new GetResponse stub. The stub itself isn't documented however, so Sphinx doesn't know where GetResponse should point to.

Something like this fixes it:

diff --git a/docs/panels.rst b/docs/panels.rst
index db4e9311..f2fe35c0 100644
--- a/docs/panels.rst
+++ b/docs/panels.rst
@@ -352,6 +352,8 @@ There is no public CSS API at this time.

     .. automethod:: debug_toolbar.panels.Panel.run_checks

+.. autoclass:: debug_toolbar._stubs.GetResponse
+
 .. _javascript-api:

 JavaScript API

I'm not sure if that's what we want. Maybe there's a different way to specify the type which doesn't require the GetResponse class to be documented as well? Or maybe documenting the stub itself (as a temporary measure, until Django ships type hints?) would be the way to go?

@leandrodesouzadev
Copy link
Contributor Author

Will definitely take a look into it

@tim-schilling
Copy link
Member

@leandrodesouzadev 👋 I wanted to check in to see how this is coming along. Did you want to keep moving forward with it?

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.

Add Type hints to the codebase
3 participants