-
Notifications
You must be signed in to change notification settings - Fork 55
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
Clean Up Dead Code #1132
Clean Up Dead Code #1132
Conversation
- id: pytest-check | ||
name: pytest-check | ||
# Needed because otherwise it seems to always run pytest in the git root. | ||
entry: poetry run pytest lib/sycamore/sycamore/tests/unit apps/crawler/crawler/s3/tests/unit |
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 looks like it removes the sycamore unit tests as well? Do you not just want to remove the crawler part?
I don't use pre-commit any more, so not sure if this works at all, but if you are keeping it it seems like it makes sense to run the sycamore unit tests.
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.
Good catch, changed.
apps/weaviate/compose.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.
Did we decide to remove this? Can't remember. I have used it when testing Weaviate. Is it used in the integ tests?
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 was used to test if the one in the docs (https://sycamore.readthedocs.io/en/stable/sycamore/connectors/weaviate.html) works correctly, so we don't need this any more.
Query-eval stuff looks good to go, can merge after Ben's comments |
No description provided.