Skip to content

feat: Add unit tests for SplitRecursively function #348

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 25, 2025
Merged

feat: Add unit tests for SplitRecursively function #348

merged 2 commits into from
Apr 25, 2025

Conversation

FranFigueroa
Copy link
Contributor 

    Adds unit tests covering basic functionality, overlap, whitespace trimming,
    and discarding empty chunks for the SplitRecursively function.

    Includes TODOs noting potential inconsistencies observed during testing
    regarding overlap and whitespace handling for further investigation.

    Closes #34

@FranFigueroa
feat: Add unit tests for SplitRecursively function
db30c54 
        Adds unit tests covering basic functionality, overlap, whitespace trimming,
        and discarding empty chunks for the SplitRecursively function.

        Includes TODOs noting potential inconsistencies observed during testing
        regarding overlap and whitespace handling for further investigation.

        Closes cocoindex-io#34
badmonster0
badmonster0 reviewed Apr 20, 2025
View reviewed changes
Copy link
Member

@badmonster0 badmonster0 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for making this PR! Really appreciate the effort!

Would you mind translating the comments to English? Since English is used throughout codebase of this project. Thanks!

@FranFigueroa
Copy link
Contributor Author 

Hi!

Thanks a lot for the review and the feedback! I really appreciate you taking the time.

I've updated the PR to translate the comments into English as requested. The latest commit reflects this change.

Let me know if any other changes are needed!

badmonster0
badmonster0 reviewed Apr 20, 2025
View reviewed changes
@FranFigueroa FranFigueroa force-pushed the add-split-recursively-tests branch from 2e10170 to 6b75569 Compare April 20, 2025 18:21
@FranFigueroa
Copy link
Contributor Author

My apologies for missing those earlier!

I've removed non-English comments. The code should now only contains comments in english, as requested.

Please let me know if everything looks good now or if there's anything else needed.

Thanks!

@FranFigueroa FranFigueroa force-pushed the add-split-recursively-tests branch from 6b75569 to a372f41 Compare April 20, 2025 19:56
@badmonster0 badmonster0 mentioned this pull request Apr 20, 2025
Closed
badmonster0
badmonster0 reviewed Apr 20, 2025
View reviewed changes
@FranFigueroa FranFigueroa force-pushed the add-split-recursively-tests branch from a372f41 to 98a0f3c Compare April 25, 2025 22:04
@badmonster0 badmonster0 merged commit 0be50d6 into cocoindex-io:main Apr 25, 2025
2 checks passed
@badmonster0
Copy link
Member

@FranFigueroa we just published release note and made a section for you https://cocoindex.io/blogs/cocoindex-changelog-2025-04-30#franfigueroa. we love your contribution and welcome to the community!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@badmonster0 badmonster0 badmonster0 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FranFigueroa @badmonster0