Skip to content

Conversation

@rwb27
Copy link
Collaborator

@rwb27 rwb27 commented Jun 12, 2025

This adds some higher-level descriptions of concurrency, Blob objects, and "client" code, as well as the beginning of a tutorial. Currently, the tutorial covers installing and running the library, but not how to write a Thing. Completing the tutorial would be a very good idea.

It would be lovely to get some sort of doctest running: even if all it does is check the code doesn't error, it would guard against these docs going stale when the API changes.

@rwb27
Copy link
Collaborator Author

rwb27 commented Jun 12, 2025

It might be helpful here to build the docs from the branch. There's an action that might do this with very little effort here:
https://github.com/marketplace/actions/sphinx-build

Copy link
Contributor

@julianstirling julianstirling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have skimmed this and made a few comments. I find reading RestructuredText very hard. It would be good if we could compile docs with Actions for reading.

Copy link
Collaborator Author

@rwb27 rwb27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've addressed all of these comments, except the previews of documentation in merge requests. That ought to be possible, but I'll leave it outstanding for now.

@rwb27
Copy link
Collaborator Author

rwb27 commented Jun 16, 2025

I'm at a loss as to why the coverage job is failing. It seems to have all the secrets set as per the instructions - I can only guess I might need to re-install it?

This should be taken care of by reathedocs.io
@rwb27
Copy link
Collaborator Author

rwb27 commented Jun 19, 2025

It turns out readthedocs can automatically rebuild for PRs, we just needed to re-sync the web hook. This is now done, and the build docs for this PR are visible at
https://labthings-fastapi--106.org.readthedocs.build/en/106/

There's even a handy diff view!

Copy link
Contributor

@julianstirling julianstirling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am merging this now. I have skimmed through everything, and there is a lot more information here. I think we really need to use these docs during dev to understand what needs to be clarified.

The pipeline is failing on the coverage checker because of keys. This is because barecheck can't comment because this PR is from your personal account rather than from the LabThings one. The actual coverage change is -0.07%, which is probably because one of the python doc strings is somehow counted as line of code which is not covered.

@julianstirling julianstirling merged commit b9d5929 into labthings:main Jun 22, 2025
10 of 11 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