-
-
Notifications
You must be signed in to change notification settings - Fork 108
Improve pipeline documentation with clearer structure and practical examples #350
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
Improve pipeline documentation with clearer structure and practical examples #350
Conversation
Co-authored-by: nijel <[email protected]>
Co-authored-by: nijel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some parts are now duplicated (for example, parameter descriptions).
docs/pipeline.rst
Outdated
| in the dict is populated. In cases where the authentication is purely external, a | ||
| pipeline method must be provided that populates the ``user`` key. Example:: | ||
| **Note:** This example removes ``get_username`` and ``create_user`` steps, so only | ||
| users who have previously authenticated can log in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using .. note:: syntax might be more appropriate.
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| To override the default parameter name just define:: | ||
| ``@partial`` stores the pipeline state in a database table named ``social_auth_partial``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really a backend-specific information, this is valid for Django only.
docs/pipeline.rst
Outdated
| 2. **Place it in an importable location** in your project | ||
| 3. **Add it to the pipeline** in your settings at the appropriate position | ||
|
|
||
| **Important:** Function placement matters! The order determines what data is available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Sphinx markup instead of manual tagging.
docs/pipeline.rst
Outdated
| The pipeline functions will get quite a lot of arguments, ranging from the | ||
| backend in use, different model instances, server requests and provider | ||
| responses. To enumerate a few: | ||
| **Tip:** Always include ``**kwargs`` to handle additional parameters from other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another place to use sphinx markup.
|
@copilot Please address the code review. Use Sphinx markup when possible and avoid too long documentation with repeating the same content. |
|
Rendered docs for review: https://python-social-auth--350.org.readthedocs.build/en/350/pipeline.html |
|
@copilot Retry |
…ation Co-authored-by: nijel <[email protected]>
Addressed all code review feedback in commit 100cfac:
Documentation now builds successfully and renders properly with styled admonitions. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR significantly enhances the pipeline documentation by expanding it from basic explanations to comprehensive, tutorial-style documentation. The changes improve developer understanding of how to use and extend the pipeline system in python-social-auth.
Key changes:
- Added detailed explanations of pipeline concepts, return values, and function parameters
- Expanded examples with practical code snippets and use cases
- Added new sections on partial pipelines, custom pipeline functions, and common patterns
Comments suppressed due to low confidence (1)
docs/pipeline.rst:1
- The documentation states
current_partial.backendcontains 'The backend name' but the code example assigns it to a variable namedbackend. This is misleading as it suggests it's a backend instance rather than a backend name string. Consider clarifying in the bullet point that this is the backend name string, not the backend instance object.
Pipeline
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci


Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.