Created attachment 221935 [details] [review]
vm-creator: Ensure post install config is set after launch

We were setting post-installation config before launching the domain in
for non-express installation.

Review of attachment 221935 [details] [review]:

how does it change post-install before launch order here? it looks like post-install was called after any launch (direct or via app.select_item())

.

(In reply to comment #2)
> Review of attachment 221935 [details] [review]:
> 
> how does it change post-install before launch order here? it looks like
> post-install was called after any launch (direct or via app.select_item())

We were launching the domain (via app.select_item()) in a signal handler lambda.

Attachment 221935 [details] pushed as 22cbe3d - vm-creator: Ensure post install config is set after launch

(In reply to comment #0)
> SSIA

Once again, the subject doesn't say it all, and unfortunately, this time the commit log is not helpful either :-/ What is missing there is why is this commit needed in addition to explaining what it's doing? Is it fixing a bug, if so how does it trigger? If not, is it to make the code nicer, more correct, ...? This takes 5 minutes to add, and is very helpful to the code reviewer, and will also be really helpful 6 months from now when someone (including you) looks at this commit while refactoring code, fixing bugs, ...

(In reply to comment #5)
> (In reply to comment #0)
> > SSIA
> 
> Once again, the subject doesn't say it all, and unfortunately, this time the
> commit log is not helpful either :-/ What is missing there is why is this
> commit needed in addition to explaining what it's doing? Is it fixing a bug, if
> so how does it trigger? If not, is it to make the code nicer, more correct,
> ...? This takes 5 minutes to add, and is very helpful to the code reviewer, and
> will also be really helpful 6 months from now when someone (including you)
> looks at this commit while refactoring code, fixing bugs, ...

You seem to think that I intentionally do not provide better explaination. We are going into a feature freeze and I have been working day and night to make it happen. I'm very sorry if some commits do not have 2 page explanation. This commit is a few lines and if you read carefully, it really should be obvious whats going on.

(In reply to comment #6)
> You seem to think that I intentionally do not provide better explaination. 

No need to get personal here, unless you want the discussion to get heat up and get more emotional. Especially when your assumptions are unbased and wrong. All I was thinking is that some important information is missing from the commit log, and this comment was only a reminder with the hope that at some point, such explanations will always be there in the first patch iterations.

> We are going into a feature freeze and I have been working day and night to make
> it happen. 

This commit does not seem to be impacted by any of the freezes. Maybe it would be obvious that it's impacted with an explanation as to why we need this...

> I'm very sorry if some commits do not have 2 page explanation. This
> commit is a few lines and if you read carefully, it really should be obvious
> whats going on.

I'm not asking about what it's doing, I'm asking _why_ this needs to be done, about what led you to make this change in the first place(was there a bug? is it needed by something else? is it cleaner, more correct, .. ?). Something that is obvious now will be totally non-obvious and mysterious one month or 3 months from now.

All in all, this comment of yours starts by making unbased assumptions about my motivations for asking for this information (answer: I'm just pointing out some missing information that would make the code better/more maintainable in the long run), and then you give a list of reasons as to why you did not put this information in this commit, and finally 'just read the commit, this will answer your questions' instead of providing a straight answer... (maybe I've already done so and still don't know?)

Wouldn't it have been quicker and better for everyone involved to skip that comment which does not add anything, and to just add the missing piece of information to the bug report instead of each of us spending time writing comments there?