Re: [libvirt] [libvirt-glib] Add gvir_domain_get_saved()
On Thu, Feb 16, 2012 at 11:01:45PM +0000, Daniel P. Berrange wrote:
On Thu, Feb 16, 2012 at 10:16:11PM +0100, Christophe Fergeau wrote:
> On Thu, Feb 16, 2012 at 09:51:34AM +0100, Christophe Fergeau wrote:
> > On Thu, Feb 16, 2012 at 02:26:02AM +0200, Zeeshan Ali (Khattak) wrote:
> > > + */
> > > +gboolean gvir_domain_get_saved(GVirDomain *dom)
> >
> > The naming needs to be more explicit, libvirt will suspend the domain after
> > a call to virDomainSave or virDomainManagedSave, the current name only
> > checks for the latter state. I'd go for
> > gvir_domain_has_managed_save_image();
>
> I see this patch has been pushed to master with this part unchanged and no
> discussion whatsoever on the list, what happened there?
Oh, I ACK'd his new patch, without noticing your message in this
thread.
And I had missed the other thread too :)
Zeeshan, it's up to the patch submitter to keep track of the comments
raised during review, and to make sure all comments are addressed before
committing. When several people comment on a patch, you cannot push your
changes before reaching a consensus from everyone on the latest version of
your patch.
You generally have 2 ways of addressing a reviewer comment:
* either you agree that his suggested changes improve the code, and you
make these changes
* either you think your way is better, in which case you explain why you
did things this way and why you think this is better. Then, you need to
wait and see if the reviewer got convinced to go your way, or if he has
more objections/explanations. In which case we begin another iteration.
This process helps to get the code and the design of the library as good
and consistent as possible.
Hope that helps,
Christophe
Attachments:
- attachment.sig (application/pgp-signature — 198 bytes)