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

Add wsproto.Connection reference #188

Merged
merged 4 commits into from
Feb 24, 2024
Merged

Add wsproto.Connection reference #188

merged 4 commits into from
Feb 24, 2024

Conversation

MtkN1
Copy link
Contributor

@MtkN1 MtkN1 commented Feb 7, 2024

Hello! I have made some minor document corrections.

Summary

  1. Add wsproto.Connection reference
    wsproto.Connection is described in Post handshake connection, but it is not listed in the API reference in the documentation. It would be useful to see this in the documentation. Added autodoc directives to api.rst and fixed docstrings.

  2. Fix Read the Docs build
    While working on the above fix, I noticed that the latest build of Read the Docs for wsproto is failing.

    https://readthedocs.org/projects/wsproto/builds/22942691/

    Problem in your project's configuration. No default configuration file found at repository's root. See https://docs.readthedocs.io/en/stable/config-file/

    It looks like the .readthedocs.yaml file is now required to build Read the Docs 1. The .readthedocs.yaml file has been added. Also added requirements.txt for the documentation, since the sphinx-rtd-theme is not automatically applied.

    I don't think the Read the docs preview in the pull request is set up, so I built the docs in the fork repository. You can check that here
    https://improved-octo-garbanzo.readthedocs.io/en/latest/api.html#wsproto.Connection

Footnotes

  1. https://blog.readthedocs.com/migrate-configuration-v2/

@MtkN1
Copy link
Contributor Author

MtkN1 commented Feb 21, 2024

Hello, maintainers. Is it possible to get a review? It is not a high priority :)

@Kriechi
Copy link
Member

Kriechi commented Feb 23, 2024

LGTM, but some minor linting issues and an issue with sdist packaging of the new config file, please see the CI failure log.

@MtkN1
Copy link
Contributor Author

MtkN1 commented Feb 24, 2024

Oh, sorry. I thought the CI error was unrelated since it was also occurring in the base branch.

I have fixed this. I think the CI that is failing now is the same as the previous pull request (#187 (comment)).

@Kriechi Kriechi merged commit c3e407e into python-hyper:main Feb 24, 2024
6 of 8 checks passed
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