<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 11/15/2013 05:03 PM, Tomas Babej
wrote:<br>
</div>
<blockquote cite="mid:528645E9.6050706@redhat.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 11/07/2013 05:25 PM, Ana
Krivokapic wrote:<br>
</div>
<blockquote cite="mid:527BBF0E.6070902@redhat.com" type="cite">
<pre wrap="">Hello,
This patch addresses ticket <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/3790">https://fedorahosted.org/freeipa/ticket/3790</a>.
</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
Freeipa-devel mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre>
</blockquote>
<br>
Looking good..<br>
<br>
I have two questions:<br>
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<p> <br>
1.) Nitpick: I'd suggest we rename the save_state(service) and
restore_state(service) to more descriptive
save_service_state/restore_service_state?<br>
</p>
</blockquote>
<br>
Well, if the argument you are passing to these function does not
have a name which suggests it is a service (which it should have
anyway), you can do: `save_state(service=x)`. So I don't think
`save_service_state(x)` is more readable.<br>
<br>
<blockquote cite="mid:528645E9.6050706@redhat.com" type="cite">
<p> </p>
<p> 2.) There are other places in ipa-client-install where we save
and restore the state of the service. Having abstracted that
into a function, should we use this at other places as well?</p>
<meta name="Description" content="Copy-Paste Buffer">
<meta name="Generator" content="Zim">
<br>
</blockquote>
<br>
I looked at other instances in ipa-client-install where service
states are saved and restored. It seems that each of these cases
includes some custom logic which does not make it possible to use
the two functions I've added. <br>
<br>
<blockquote cite="mid:528645E9.6050706@redhat.com" type="cite"> <br>
<pre class="moz-signature" cols="72">--
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org</pre>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="80">--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.</pre>
</body>
</html>