Skip to content

Add Video Maker Executor script#147

Open
Nexorix-cell wants to merge 1 commit into
calesthio:mainfrom
Nexorix-cell:patch-1
Open

Add Video Maker Executor script#147
Nexorix-cell wants to merge 1 commit into
calesthio:mainfrom
Nexorix-cell:patch-1

Conversation

@Nexorix-cell

Copy link
Copy Markdown

Implement Video Maker Executor with argument parsing and logging.

Implement Video Maker Executor with argument parsing and logging.
@Nexorix-cell Nexorix-cell requested a review from calesthio as a code owner June 23, 2026 15:01

@rajpratham1 rajpratham1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This PR introduces a large new run.py (566 lines) that becomes a central execution entry point, but there are no accompanying tests or documentation updates to verify the expected behavior. For a script that handles CLI parsing, dependency validation, dynamic module loading, rendering, and error handling, I'd expect at least some validation of the new workflow before merging.

@rajpratham1 rajpratham1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This PR introduces a very large standalone run.py script (566 new lines) that duplicates responsibility already present in the project's tooling and execution flow. While the script is feature-rich and well documented, adding a new entry point of this size without tests or integration into the existing architecture makes it difficult to maintain.

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