Cole Robinson <crobinso@xxxxxxxxxx> writes: > Please use [PATCH v2] for version 2 of a patch, etc. git format-patch > --subject-prefix "PATCH v2" thanks, I'll do it. > This seems overkill. Why can't we add an argument to show() which skips the > reset_state step? > > Hmm, I guess someone could reopen the connect wizard while the first > connection is opening, which would mean the state is wrong when we re-show the > wizard. > > Thinking some more, what we really want here is: > > Launch 'Open Connection', set parameters, hit 'connect' > 'Open Connection' window is desensitized, async progress meter dialog opens up > while connection is connecting > If connection fails, we drop back to the still open connection wizard with > same state still listed. > > That's how the create/clone/delete wizards work, basically make the operation > synchronous. However truth be told I don't care enough about this bug to > mandate that we do that. If you want to look into it it might be simple, or it > might be a pain. > > But the above solution is definitely not ideal since it requires duplicating > update of UI elements in two places (set_state and reset_state) with the > latter in a rarely tested code path. I have firstly implemented it because I wanted to take care of that case but I agree it is a very uncommon case. Then I realized that set_state can also be useful if we intent to support "Modify connection" in future (not really sure it is useful). Would you accept the patch if I combine reset_state in set_state, so that set_state will accept a set_to_default argument? If you still think that the cost of maintaining the new code is bigger than the advantage of fixing that particular case, I can drop set_state. Thanks, Giuseppe _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
