Add in-progress commit status change indicator #566
Conversation
…cate the uncommitted/committed UI changes
To QAUse a real environment (the slower the better)
|
|
👍 QA okay in LXC and EC2 |
|
Test PASSed. |
| @type {String} | ||
| @default undefined | ||
| */ | ||
| commitStatus: {}, |
There was a problem hiding this comment.
should this not have an initial value?
There was a problem hiding this comment.
I had left this empty so that it would fail spectacularly if it wasn't set :) I could add a default if you'd prefer. Also I'm not sure what the default should be...committed is probably the most common.
There was a problem hiding this comment.
Hmm, I see what you mean. Most of the code isn't looking at this value so there's a lot of existing code that would have to know to create models with the right value. Setting a value means this has a chance to be a lie.
The more I think on it the more this might be right. I do think it's odd that the comments mention an assumed default if undefined.
I'll make some other suggestions based on sticking with an undefined default.
There was a problem hiding this comment.
Hmm, I see what you mean. Most of the code isn't looking at this value so there's a lot of existing code that would have to know to create models with the right value. Setting a value means this has a chance to be a lie.
The more I think on it the more this might be right. I do think it's odd that the comments mention an assumed default if undefined.
I'll make some other suggestions based on sticking with an undefined default.
There was a problem hiding this comment.
Hmm, I see what you mean. Most of the code isn't looking at this value so there's a lot of existing code that would have to know to create models with the right value. Setting a value means this has a chance to be a lie.
The more I think on it the more this might be right. I do think it's odd that the comments mention an assumed default if undefined.
I'll make some other suggestions based on sticking with an undefined default.
There was a problem hiding this comment.
Hmm, I see what you mean. Most of the code isn't looking at this value so there's a lot of existing code that would have to know to create models with the right value. Setting a value means this has a chance to be a lie.
The more I think on it the more this might be right. I do think it's odd that the comments mention an assumed default if undefined.
I'll make some other suggestions based on sticking with an undefined default.
|
👍 with a couple of notes/clarifications and the suggestion we revisit the idea of an undefined default on the model value. |
|
Thanks for the reviews all! |
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge |
Add in-progress commit status change indicator When deploying a changeset there is a time between when the ecs sends the command and juju ack's it. This time can be quite large on large deployments and on slow environments. This now changes the blue circle in the machine view to a yellow one until the juju ack.
|
Test PASSed. |
|
Test PASSed. |
When deploying a changeset there is a time between when the ecs sends the command and juju ack's it. This time can be quite large on large deployments and on slow environments. This now changes the blue circle in the machine view to a yellow one until the juju ack.