Skip to content

Close possibly open objects #985

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

Merged
merged 2 commits into from
Apr 30, 2024
Merged

Conversation

abhishek-compro
Copy link
Contributor

This is mostly a refactor. Let me know if there are any conflicting opinions.

@weaviate-git-bot
Copy link

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

@abhishek-compro
Copy link
Contributor Author

@weaviate-git-bot I agree to the Contributor License Agreement

@abhishek-compro
Copy link
Contributor Author

abhishek-compro commented Apr 3, 2024

This is a very simple change but looks complicated in the default diff view. After you enable Hide whitespace in the diff view settings on GitHub you will be able to view only the lines that were added and removed.

@dirkkul
Copy link
Collaborator

dirkkul commented Apr 10, 2024

HI @abhishek-compro , thanks for your contribution! There are some type-checking failures, could you please adress them?

@abhishek-compro
Copy link
Contributor Author

HI @abhishek-compro , thanks for your contribution! There are some type-checking failures, could you please adress them?

Done

@abhishek-compro
Copy link
Contributor Author

@dirkkul I have run the tests as per the contributing guide and it only gives some warnings but no errors. I am using version 1.24.1.

https://github.com/weaviate/weaviate-python-client/actions/runs/8684237813/job/23851552307?pr=985#step:6:1286

I don't know how to fix address that is already in use. Is it something specific to my changes?

@dirkkul dirkkul merged commit fd0fd90 into weaviate:main Apr 30, 2024
39 of 40 checks passed
@dirkkul
Copy link
Collaborator

dirkkul commented Apr 30, 2024

I don't know how to fix address that is already in use. Is it something specific to my changes?

that is sadly a flaky CI test, should not be connected to your changes

Thanks for your contribution, merged :)

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.

3 participants