Skip to content
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

Feature/handle live query diff #105

Merged
merged 12 commits into from
Nov 28, 2023

Conversation

Atoo35
Copy link
Contributor

@Atoo35 Atoo35 commented Oct 17, 2023

@phughk i am pretty sure i have done something horrible in here. guide me please

This PR resolves issue #81

Copy link
Contributor

@phughk phughk left a comment

Choose a reason for hiding this comment

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

Left a comment, but would be good to get @timpratim s eyes on this as well. Ex the deserialisation. I am creating a ticket to move it out of gorilla and test. Otherwise I think it's ok, thanks for fixing!

db.go Outdated Show resolved Hide resolved
db_test.go Show resolved Hide resolved
@Atoo35
Copy link
Contributor Author

Atoo35 commented Oct 21, 2023

@phughk i have fixed like you suggested. Although not sure why the test is failing here on github, being a developer "it worked just fine on my machine" !!

@ilkerkorkut
Copy link

@phughk i have fixed like you suggested. Although not sure why the test is failing here on github, being a developer "it worked just fine on my machine" !!

Most probably it's related with that issue -> #75

@ElecTwix
Copy link
Contributor

@phughk i have fixed like you suggested. Although not sure why the test is failing here on github, being a developer "it worked just fine on my machine" !!

that issue was fixed with #109
You can continue your work :)

@Atoo35
Copy link
Contributor Author

Atoo35 commented Nov 22, 2023

thanks @ElecTwix the work was done already, was just stuck in the test pipeline here. @phughk is this fine to be merged now?

Copy link
Contributor

@phughk phughk left a comment

Choose a reason for hiding this comment

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

Happy to approve once the param is changed.
Thanks for this @Atoo35

db.go Outdated Show resolved Hide resolved
@Atoo35
Copy link
Contributor Author

Atoo35 commented Nov 27, 2023

@phughk made the change as requested. Thanks a lot !

Copy link
Contributor

@phughk phughk left a comment

Choose a reason for hiding this comment

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

Lgtm thanks @Atoo35 !

@phughk phughk merged commit 159d4a6 into surrealdb:main Nov 28, 2023
2 checks passed
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.

4 participants