Ana Krivokapic wrote:
On 11/15/2013 05:03 PM, Tomas Babej wrote:
On 11/07/2013 05:25 PM, Ana Krivokapic wrote:
Hello,
This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/3790.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Looking good..
I have two questions:
1.) Nitpick: I'd suggest we rename the save_state(service) and
restore_state(service) to more descriptive
save_service_state/restore_service_state?
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.
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?
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.
It looks fine to me as-is. I couldn't find any other services to include
in this which is indeed a shame as this significantly simplifies things.
ACK
rob
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
