Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core/managed): add proper support for skipped, approved artifact states #8142

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

erikmunson
Copy link
Member

We recently came to the realization that the approved and pending states an artifact can have in a particular environment were a bit conceptually overloaded, because they both contained versions that had been skipped over due to later versions becoming available. This meant it was very challenging to explain what approved and pending meant in the UI because a version could either be in a state of approved-ness / pending-ness, or it could've been skipped and have no chance of ever being deployed.

We've now split out a totally new stateskipped — to help clarify the semantics of the other two states, freeing up approved to mean "this version is about to start deploying" and pending to mean "this version is actively under consideration for deployment, but hasn't been chosen yet". This change adds full-fledged support to all three of these statuses, as the inherent confusion had prevented us from adding useful UI treatments until this point. Looks like this:

Pending:
Screen Shot 2020-04-07 at 7 58 05 PM

Approved:
Screen Shot 2020-04-07 at 7 41 52 PM

Skipped (version info available):
Screen Shot 2020-04-07 at 7 29 53 PM

Skipped (no version info available):
Screen Shot 2020-04-07 at 7 37 56 PM

Icons are coming soon (the approved one is also temporary), but wanted to get this out in advance of having iconography.

(cc @gcomstock)

Copy link
Contributor

@vigneshm vigneshm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@erikmunson erikmunson merged commit b7709ab into spinnaker:master Apr 8, 2020
@erikmunson erikmunson deleted the skipped-state-support branch April 8, 2020 17:24
@luispollo
Copy link
Contributor

❤️ it!

For another PR, I still think we could change "Already deployed to previous environment X" to "Dependency on environment X passes" or "Dependency on environment X satisfied", though. 😄

@erikmunson
Copy link
Member Author

I did call that out as an alternative we very well might move to depending on how the overall experience is feeling, but for now I'm sensitive to keeping the voice and tone of the foundational/core constraint cards fairly consistent with each other. I'd likely rework the tone of some other cards along with it if we end up going that direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants