Skip to content

Conversation

@sinisaos
Copy link
Member

Related to #459

@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.04%. Comparing base (1157c12) to head (745f576).
⚠️ Report is 49 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #461      +/-   ##
==========================================
+ Coverage   93.42%   94.04%   +0.62%     
==========================================
  Files           5        5              
  Lines         365      403      +38     
==========================================
+ Hits          341      379      +38     
  Misses         24       24              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

github-actions bot commented Dec 1, 2025

This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed?

@github-actions github-actions bot added the Stale label Dec 1, 2025

The docs are written using Sphinx. To get them running locally:

* Install the requirements: ``pip install -r requirements/readthedocs-requirements.txt``
Copy link
Member

Choose a reason for hiding this comment

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

Could this just be requirements/doc-requirements.txt instead of requirements/readthedocs-requirements.txt?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. If we run make html with previous pip install -r requirements/doc-requirements.txt, we will get ModuleNotFoundError

(doc_env) rkl@mint:~/dev/doc_env/piccolo_admin/docs$ make html
Running Sphinx v7.3.7

Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/home/rkl/dev/doc_env/lib/python3.12/site-packages/sphinx/config.py", line 509, in eval_config_file
    exec(code, namespace)  # NoQA: S102
    ^^^^^^^^^^^^^^^^^^^^^
  File "/home/rkl/dev/doc_env/piccolo_admin/docs/source/conf.py", line 29, in <module>
    import piccolo_admin  # noqa
    ^^^^^^^^^^^^^^^^^^^^
  File "/home/rkl/dev/doc_env/piccolo_admin/piccolo_admin/__init__.py", line 1, in <module>
    from .endpoints import FormConfig, create_admin
  File "/home/rkl/dev/doc_env/piccolo_admin/piccolo_admin/endpoints.py", line 19, in <module>
    import typing_extensions
ModuleNotFoundError: No module named 'typing_extensions'

make: *** [Makefile:20: html] Error 2

With readthedocs-requirements.txt we don't have this problem because we use full requirements in that file

Copy link
Member

Choose a reason for hiding this comment

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

I see your point. Can we do this instead:

* Make sure the main requirements are installed: ``pip install -r requirements/requirements.txt``
* Install the documentation requirements: ``pip install -r requirements/requirements.txt``

I know it sounds weird, but I don't want to reference the readthedocs requirements, just because I want to keep that just for readthedocs, and we might move away from them in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@dantownsend
Copy link
Member

Looks great, thanks!

@dantownsend dantownsend merged commit 7c69df9 into piccolo-orm:master Dec 17, 2025
11 checks passed
@sinisaos sinisaos deleted the contributing_guide_for_docs branch December 17, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants