-
-
Notifications
You must be signed in to change notification settings - Fork 2
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 basic sphinx & readthedocs infra #223
Conversation
current RTD fail is missing dependencies to be able to import flake8_async, so need to install those, or scraping the |
Nice. Now I just gotta get some docs |
Slowly closing in on this being an improvement/on-par to status quo. |
Okay I think this is worth reviewing now. You can view the docs at https://flake8-async--223.org.readthedocs.build/en/223/ (or click through from the CI RTD check) Other minor TODOs:
But I think we can put those in a separate PR. Would be nice to get this merged so I can update docs/ when writing feature PRs. |
docs/usage.rst
Outdated
************* | ||
|
||
`You can configure flake8 with command-line options <https://flake8.pycqa.org/en/latest/user/configuration.html>`_, | ||
but we prefer using a config file. The file needs to start with a section marker ``[flake8]`` and the following options are then parsed using flake8's config parser, and can be used just like any other flake8 options. |
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.
Specify allowed filenames / types?
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.
rewrote/expanded it, referring to flake8 documentation for filenames/types/section marker. Also linking pyproject-flake8, and suggesting various ways of getting around us not having config support for running as a standalone.
|
||
- **ASYNC100**: A ``with [trio/anyio].fail_after(...):`` or ``with [trio/anyio].move_on_after(...):`` context does not contain any ``await`` statements. This makes it pointless, as the timeout can only be triggered by a checkpoint. This check also allows ``yield`` statements, since checkpoints can happen in the caller we yield to. |
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.
Let's add a "General rules" subheading for the 1xx rules.
I'd also consider using a sub-subheading for each individual rule (instead of the bullet points), so that it's easier to add code examples etc. That's probably better as a follow-up PR though.
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.
Let's add a "General rules" subheading for the 1xx rules.
👍
I'd also consider using a sub-subheading for each individual rule (instead of the bullet points), so that it's easier to add code examples etc. That's probably better as a follow-up PR though.
It's either that or a separate subpage for each one (ruff style) as you suggested in #214 (comment)
I'm not sure which one I prefer, tons of small pages can be messy to navigate - so maybe a dense list for an overview that then links to longer sections for each rule. A raw TOC wouldn't be great with error codes, but we probably should come up with human-friendly names for the codes at some point (astral-sh/ruff#1773)
But yeah, leaving that one for followup as I said in #223 (comment)
docs/rules.rst
Outdated
- **TRIOxxx**: All error codes are now renamed ASYNCxxx | ||
- **TRIO107**: Renamed to TRIO910 | ||
- **TRIO108**: Renamed to TRIO911 | ||
- **TRIO117**: Don't raise |
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.
Incomplete sentence.
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.
smh, that's Claude randomly deciding to stop giving output without any warning 🙃
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.
See comments above, happy for you to merge this when you think it's ready and follow up later.
Co-authored-by: Zac Hatfield-Dodds <[email protected]>
Let's see if this works~