-
-
Notifications
You must be signed in to change notification settings - Fork 871
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
Documentation lagging behind? #343
Comments
Maybe we need to be mindful of creating documentation issues before the feature PR lands and linking them together. Within the PR's life-cycle we can ask the author if they want to handle the docs post-merge otherwise it can be taken up by maintainers. I think for maintainers it's reasonable to expect documentation within the PR that adds a feature requiring documentation. |
I'd prefer requiring documentation changes as part of the PR, I feel I could forget otherwise (and indeed I have forgotten with the Digest auth). If maintainers feel the PR is being delayed we can offer help or maybe push commits with small fixes. |
I'm also in favor of docs-with-changes, as they can get missed otherwise and are easier to write when the code it's documenting is fresh in the contributor's mind. |
We should roll with docs-with-changes as an absolute requirement, always. If anything I'd love to see more pull requests that start with the documentation, and then progressively add commits for the code, only once any changes have been discussed from the POV of the documented functionality. |
So, I don’t think this issue is very actionable in itself so I’d be in favor of closing it for housekeeping purposes. At least I think we agreed on this: let’s keep in mind that documentation changes should be part of a PR that touches anything publicly exposed. :) |
There are a few pieces of public functionality that are not properly documented yet.
For example:
async with
syntax for theAsyncClient
(see request.close or AsyncClient.close? #342) — I'll push a PR for this soon.auth
parameter is documented in the Developer Interface, but there's no usage guide).This issue is more to strike a discussion about how we should handle documenting features as they are merged into master.
Including documentation as part of the PR has been working very well for me in the past, although it does increase the amount of content to review. The main reason is that in general it's hard to provide the initial documentation for a feature we did not implement ourselves.
Something we could keep in mind is to ask ourselves "does this require a documentation update?" for each PR. Maybe setting up a PR template with a comment hinting at this could do the trick? Of course, there's the concern of raising the barrier for contributors, but someone has to update the docs anyway, so…
Thoughts @encode/httpx-maintainers and all?
The text was updated successfully, but these errors were encountered: