[libvirt] [PATCH v2a] HyperV: Improve 2008, introduce 2012

Jason Miesionczek jmiesionczek at datto.com
Fri Sep 16 18:11:12 UTC 2016


Not sure I understand what you mean by ‘not adding the string table’. Could you please elaborate?

I understand what you are saying about splitting things up into more logical chunks, by topic, so i will do that for the patches going forward.

Thanks for your feedback, i’m still pretty new to C (not having touched it in about 15 years), so I appreciate the help. 

Also, could you elaborate on the 2-level string data vs. single level enum/index stuff? I’m not sure I follow what you are suggesting.

Thanks!

> On Sep 16, 2016, at 2:02 PM, Matthias Bolte <matthias.bolte at googlemail.com> wrote:
> 
> 2016-09-16 18:35 GMT+02:00 Jason Miesionczek <jmiesionczek at datto.com>:
>> Second round of patches based on recently complete code review. Going
>> to submit patches in much smaller chunks, starting with this one. Future
>> patches will be submitted as each previous patch is reviewed and merged.
> 
> 1-commit flow isn't ideal either.
> 
> If I could just look at commit 1 alone then I would not have
> understood how you're going to use this new 2-level string lookup
> table.
> 
> As John suggested you should try to work in smaller chunks, like 4-6
> commits at a time. And also try to keep them grouped by topic.
> 
> You should avoid having multiple independent things in one patch, like
> the new types and the table generation or the autostart functions and
> the general invoke functions. Do one thing per commit and ensure that
> each commit can stand alone, e.g. the code compiles and works cleanly
> after each commit.
> 
> For example the first commit adds some new types. The second commit
> uses these new types.
> 
> But those two commits would not add the string table. This would
> either be a separate commit or be part of the commit add the gen
> general code the uses the string table.
> 
> -- 
> Matthias Bolte
> http://photron.blogspot.com





More information about the libvir-list mailing list