Skip to content

new project structure#89

Closed
denis-samatov wants to merge 2 commits intoVectifyAI:mainfrom
denis-samatov:main
Closed

new project structure#89
denis-samatov wants to merge 2 commits intoVectifyAI:mainfrom
denis-samatov:main

Conversation

@denis-samatov
Copy link
Copy Markdown

No description provided.

@saccharin98
Copy link
Copy Markdown
Collaborator

Thanks for the effort put into this PR! Unfortunately, we're going to close it without merging.

The core issue is scope: this PR restructures the entire project in a single change — moving from pageindex/ to src/pageindex/, splitting utils.py, introducing Pydantic config, reorganizing cookbook/,
tests/, adding new scripts, and more. A change this large is very difficult to review, test, and maintain going forward. If anything breaks, it becomes hard to isolate the cause.

A few specific blockers:

  • README.md and CHANGELOG.md have been largely translated into Russian, which isn't appropriate for a public, international open-source project.
  • The src/ layout restructuring is a breaking change with no migration path for existing users.
  • local_client_adapter.py, analyze_notebooks.py, and refactor_notebooks_logic.py appear to be development-time scripts that don't belong in the main package.

That said, some of the ideas here are good — the utils.py modularization and Pydantic-based config validation are directions we'd welcome. If you'd like to contribute, we'd suggest breaking this into
smaller, focused PRs (e.g., bug fixes in utils.py, then config improvements separately) so each change can be properly reviewed.

Closing this PR. Thanks again!

@denis-samatov
Copy link
Copy Markdown
Author

Thanks for the detailed feedback and for taking the time to review the PR.

That makes sense — I understand the concern about scope. I combined too many structural and functional changes into a single PR, which made it hard to review safely and incrementally.

You’re also right about the specific blockers:

  • translating README.md and CHANGELOG.md into Russian was not appropriate for a public international project;
  • the move to the src/ layout is a breaking change and I did not provide a proper migration path;
  • some of the added scripts are development-time utilities and should not have been included in the main package PR.

I appreciate the note that parts of the work were still useful, especially the utils modularization and the Pydantic-based config direction.

I’ll take this feedback into account and, if I reopen contributions here, I’ll split them into smaller focused PRs with isolated changes and clearer review boundaries.

Thanks again for the review.

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