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

Implement a more generic parser for command line arguments #55

Closed
andreeaflorescu opened this issue Jan 11, 2021 · 6 comments · Fixed by #56
Closed

Implement a more generic parser for command line arguments #55

andreeaflorescu opened this issue Jan 11, 2021 · 6 comments · Fixed by #56
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@andreeaflorescu
Copy link
Member

We currently duplicate the parsing logic in the try_from functions of MemoryConfig, VcpuConfig, KernelConfig, NetConfig, BlockConfig. As we add more devices and configuration, this duplication generates too much boilerplate code.

We should try to see if we can somehow reduce this duplication (without adding new rust crates dependencies).

@andreeaflorescu andreeaflorescu added good first issue Good for newcomers help wanted Extra attention is needed labels Jan 11, 2021
@FedericoPonzi
Copy link

Could you please assign this to me?

@andreeaflorescu
Copy link
Member Author

@FedericoPonzi done! Happy coding!

@FedericoPonzi FedericoPonzi mentioned this issue Jan 13, 2021
@gabhijit
Copy link
Contributor

Related: I am thinking whether it's a good idea to use builder pattern here. Something along the lines of hyper Request builder?

So the code would look like -

let vmm_cfg = VMMConfig::builder()
.memory(...)
.kernel(...)
.net(...)

And with right set of defaults, so not everything becomes a required config?

@andreeaflorescu
Copy link
Member Author

Related: I am thinking whether it's a good idea to use builder pattern here. Something along the lines of hyper Request builder?

So the code would look like -

let vmm_cfg = VMMConfig::builder()
.memory(...)
.kernel(...)
.net(...)

I personally like the builder pattern, I wonder how much it helps in this scenario. A prototype would be nice.

And with right set of defaults, so not everything becomes a required config?

Besides the kernel path, for everything else we can have defaults.

@gabhijit are you interested in contributing your proposal?

@gabhijit
Copy link
Contributor

@andreeaflorescu , Sure I can give it a try . Let me create a separate issue?

@andreeaflorescu
Copy link
Member Author

@andreeaflorescu , Sure I can give it a try . Let me create a separate issue?

Yap, sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants