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

Add method that allows user to specify DataDir directly #99

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

Luoxin
Copy link
Contributor

@Luoxin Luoxin commented Aug 22, 2024

Add a method that allows users to directly specify DataDir, simplifying the call path when users only want to change the DataDir value

@Luoxin
Copy link
Contributor Author

Luoxin commented Aug 22, 2024

#98

Copy link
Collaborator

@kelvinmwinuka kelvinmwinuka left a comment

Choose a reason for hiding this comment

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

@Luoxin

What's the benefit of this change over using WithConfig in the following manner?

config := echovault.DefaultConfig()
config.DataDir = "./data"
cache, err := echovault.NewEchoVault(echovault.WithConfig(config))
if err != nil {
  log.Fatal(err)
}

I think it's an interesting idea to provide an option like this for editing config. However, for consistency, we would have to provide this for all other configurable values as well, as users will expect them.

If you'd like to go this route, I'd recommend you create these options for all the config options and add them to the PR. We should also move all these options to the echovault/config.go file.

@Luoxin
Copy link
Contributor Author

Luoxin commented Sep 1, 2024

The user can complete the operation of the NewEChovault instead of adding a lot of lines directly through the line of Withxxx

I will make some modifications according to your suggestions.

@Luoxin
Copy link
Contributor Author

Luoxin commented Sep 3, 2024

I completed the method according to the existing fields in config.Config, with special handling for bool type data, which is considered true when no bool type value is passed in.

Copy link
Collaborator

@kelvinmwinuka kelvinmwinuka left a comment

Choose a reason for hiding this comment

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

Looks good!

@kelvinmwinuka kelvinmwinuka merged commit 07a5a3b into EchoVault:main Sep 6, 2024
1 check 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.

2 participants