-
Notifications
You must be signed in to change notification settings - Fork 31
Revise readthedocs content #781
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
Conversation
Reviewer's Guide by SourceryThis pull request restructures the documentation for the QM project, moving the documentation files to a new 'docs/docs' directory. It also adds a new index page, a getting started guide, and a page with additional resources. Furthermore, it updates documentation links and paths across project files to reflect the new documentation structure. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @iamianmullins - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a brief explanation of why the readthedocs structure wasn't sensible and what problems this change addresses.
- It looks like you're moving the development README - make sure that link isn't broken.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
001ba28
to
48c8050
Compare
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.
Consider running automation with github actions
48c8050
to
336d579
Compare
1185756
to
eec7802
Compare
62e68d1
to
1a40e55
Compare
@pypingou , I wonder could you help out with this. I've added this github workflow for ReadTheDocs. it's failing here, but I can see RTD successfully generated the preview. I suspect it's something to do with the webhook permissions to write to a pull request, but I don't have access to check. |
Is this a chicken-egg problem where the pipeline can't run until it's merged? From an ACL POV it looks good to me, the token has the "Pull Request" checkbox which includes: |
1a40e55
to
75d38dc
Compare
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.
Hey @iamianmullins - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a brief explanation of the rationale behind the documentation changes in the PR description.
- The addition of the RTD workflow is great, but consider testing it to ensure it functions as expected.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
75d38dc
to
91f1ea9
Compare
d569598
to
9632e5e
Compare
docs/mkdocs.yml
Outdated
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.
This should have per PR automates test into github actions, isnt it?
.github/workflows/mkdocs-check.yml
name: MkDocs Build Check
on:
pull_request:
paths:
- 'docs/**'
- 'mkdocs.yml'
- '**.md'
jobs:
mkdocs-build:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v3
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.x'
- name: Install MkDocs and dependencies
run: |
pip install \
mkdocs \
mkdocs-material \
pymdown-extensions
- name: Build MkDocs site
run: mkdocs build --strict
Consider running this
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.
Hey @iamianmullins - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using a tool like
mkdocs-redirects
to handle old links and prevent broken URLs. - Double-check the
edit_uri
inmkdocs.yml
to ensure it points to the correct location for editing the documentation files.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@iamianmullins please lets us know when the PR is ready for review/merge. |
9632e5e
to
1d965fa
Compare
Thanks for the suggestions @Yarboa , much appreciated. @dougsland this should be good for review now, thanks! |
1d965fa
to
438f3c8
Compare
- Minimize RTD content to serve as an appropriate starting point to improve structure. - Introduce mkdocs-check.yml workflow Signed-off-by: Ian Mullins <[email protected]>
438f3c8
to
252b49d
Compare
LGTM |
readthedocs isn't structured sensibly at present.
Minimizing RTD content to serve as an appropriate starting point to improve structure.
Summary by Sourcery
Restructure and improve documentation for the QM project, creating a more organized and informative documentation structure on Read the Docs.
Enhancements:
Documentation: