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

Added NewWithOptions func #51

Merged
merged 5 commits into from
Apr 6, 2023
Merged

Added NewWithOptions func #51

merged 5 commits into from
Apr 6, 2023

Conversation

ElecTwix
Copy link
Contributor

Added NewWithTimeout func for set timeout custom time
Update UnmarshalRaw returning just an error.

db.go Outdated Show resolved Hide resolved
Tests added.
Cleanup Tests.
@ElecTwix ElecTwix changed the title Added NewWithTimeout func Added NewWithOptions func Apr 4, 2023
@ElecTwix
Copy link
Contributor Author

ElecTwix commented Apr 4, 2023

  • Change the name of func to NewWithOptions
  • Modify websocket struct fields to public to access from outside for Options.

@ElecTwix ElecTwix requested review from phughk and plally April 4, 2023 16:32
@phughk phughk self-assigned this Apr 5, 2023
func NewWithOptions & func New become func New
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.

Leaving a comment in haste because I have an urgent rust thing to get to, but I think overall it's nice to have these changes thank you! I would certainly prefer converging to the single New interface though. We don't need excessive functions. I was also wondering about the visitor pattern used for the options... maybe that could be changed?

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

ElecTwix commented Apr 5, 2023

Leaving a comment in haste because I have an urgent rust thing to get to, but I think overall it's nice to have these changes thank you!

No Problem.

I would certainly prefer converging to the single New interface though. We don't need excessive functions.

I got your point still we need some struct for creating an interface method.
Still, I think we don't need more functions or make other ones excessive function.
Also maybe I'm wrong I would like to listen to what is your plan about this.

I was also wondering about the visitor pattern used for the options... maybe that could be changed?

I was thinking the user can provide the struct as a parameter then we just set the field with the original.

db.go Outdated Show resolved Hide resolved
@phughk
Copy link
Contributor

phughk commented Apr 6, 2023

This is really good work, thank you @ElecTwix !
And thank you as well for reviewing @plally it's mega helpful!

fix Unittest SurrealDBOption
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.

Amazing work @ElecTwix thank you for fixing all the issues! Really liking the shape of it now :D Merging
Shoutout to @plally for reviews!

@phughk phughk merged commit 84e33e2 into surrealdb:main Apr 6, 2023
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