Skip to content

[WIP] Fix documentation so that 'make doctest' passes#191

Merged
DanielAvdar merged 5 commits intomainfrom
copilot/fix-190
Jun 6, 2025
Merged

[WIP] Fix documentation so that 'make doctest' passes#191
DanielAvdar merged 5 commits intomainfrom
copilot/fix-190

Conversation

Copy link
Contributor

Copilot AI commented Jun 6, 2025

  • Explore repository structure and understand the codebase
  • Identify the issue: make doctest target in Makefile is incorrectly defined
  • Confirm existing documentation tests pass via pytest
  • Verify sphinx doctest builder works when run manually
  • Fix Makefile doctest target to run sphinx doctest builder
  • Create docs/source/_static directory to resolve warning
  • Add kfp_functions.py to .gitignore to prevent test artifacts
  • Test the fix to ensure make doctest passes successfully

The main fixes implemented:

  1. Updated Makefile doctest target to run sphinx-build -M doctest instead of depending on HTML doc build
  2. Created missing docs/source/_static directory to resolve sphinx warning
  3. Added generated test file to .gitignore to prevent artifacts

The make doctest command now runs successfully with 0 tests and 0 failures, meeting the acceptance criteria.

Fixes #190.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@EH-MLS
Copy link
Collaborator

EH-MLS commented Jun 6, 2025

@copilot retry

Co-authored-by: eh-main-bot <171766998+eh-main-bot@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 6, 2025

@copilot retry

Fixed the doctest target in commit 1d065e2. The make doctest command now runs the sphinx doctest builder successfully with 0 tests and 0 failures.

Copilot finished work on behalf of EH-MLS June 6, 2025 13:06
@EH-MLS EH-MLS marked this pull request as ready for review June 6, 2025 15:09
@EH-MLS EH-MLS requested a review from DanielAvdar June 6, 2025 15:10
Makefile Outdated
Comment on lines 29 to 30
doctest: install-docs
uv run --no-sync sphinx-build -M doctest docs/source docs/build/ -W --keep-going --fresh-env
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
doctest: install-docs
uv run --no-sync sphinx-build -M doctest docs/source docs/build/ -W --keep-going --fresh-env
doctest: install-docs doc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the doctest target to depend on both install-docs and doc targets as suggested. The doctest now builds HTML documentation first before running the sphinx doctest builder. Fixed in commit 93aa838.

Copy link
Owner

Choose a reason for hiding this comment

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

remove this file, also remove _static if exists

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the .gitkeep file and the entire _static directory as requested. Also updated conf.py to set html_static_path = [] to avoid sphinx warnings. Fixed in commit 93aa838.

@EH-MLS
Copy link
Collaborator

EH-MLS commented Jun 6, 2025

@copilot follow the reviews

Co-authored-by: eh-main-bot <171766998+eh-main-bot@users.noreply.github.com>
Copilot finished work on behalf of EH-MLS June 6, 2025 15:17
Makefile Outdated
uv run mypy ml_orchestrator --config-file pyproject.toml

doctest: install-docs doc
uv run --no-sync sphinx-build -M doctest docs/source docs/build/ -W --keep-going --fresh-env
Copy link
Collaborator

Choose a reason for hiding this comment

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

this line is unnecessary @copilot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the unnecessary doctest command line as requested. The doctest target now only depends on install-docs and doc targets. Fixed in commit 5f3a6e8.

Co-authored-by: eh-main-bot <171766998+eh-main-bot@users.noreply.github.com>
Copilot finished work on behalf of EH-MLS June 6, 2025 15:22
@DanielAvdar DanielAvdar merged commit d546d29 into main Jun 6, 2025
7 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.

Fix documentation so that 'make doctest' passes

3 participants