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: promote stages from freightline #736

Merged
merged 4 commits into from
Sep 13, 2023
Merged

feat: promote stages from freightline #736

merged 4 commits into from
Sep 13, 2023

Conversation

rbreeze
Copy link
Contributor

@rbreeze rbreeze commented Sep 13, 2023

Screenshot 2023-09-12 at 17 35 57 Screenshot 2023-09-12 at 17 36 08

Signed-off-by: Remington Breeze <remington@breeze.software>
@netlify
Copy link

netlify bot commented Sep 13, 2023

Deploy Preview for docs-kargo-akuity-io canceled.

Name Link
🔨 Latest commit 2c5fa74
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/65015b0849f25a0008c30736

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (c50e7bb) 46.56% compared to head (2c5fa74) 46.56%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #736   +/-   ##
=======================================
  Coverage   46.56%   46.56%           
=======================================
  Files          97       97           
  Lines        6748     6748           
=======================================
  Hits         3142     3142           
  Misses       3451     3451           
  Partials      155      155           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jessesuen
Copy link
Member

jessesuen commented Sep 13, 2023

Issue 1: I can't tell the content of what is about to be promoted

Screen.Recording.2023-09-12.at.5.45.24.PM.mov

I think the confirmation should pop up below the freight instead of ontop of it. Something like:

image

@jessesuen
Copy link
Member

Issue 2: Now that we have this UI, let's remove the availableFreight tab completely in stage view.

image

@jessesuen
Copy link
Member

jessesuen commented Sep 13, 2023

Issue 3: the freightline should be expanded by default to show more useful information:

NOTE: this is unrelated to the Promote button being added in this PR, but it has to do with Freightline in general so putting feedback here. It also may help address issue 1.

image

In the above screenshot, each box just shows freight ID (not that useful) and the same two icons for all items in the line (also not that useful). I propose we expand this by default and get rid of the pinning feature. It looks like we have enough left/right margin to compact the boxes if you are worried about freightline becoming too wide.

image

Clicking into a freight could still expand to show more metadata (git commit message, last seen, container repository). If we do this, pinning might still be useful.

@jessesuen
Copy link
Member

jessesuen commented Sep 13, 2023

Issue 4: We lost the messaging about downstream subscribers affected by the promotion.

I liked the level of detail that we previously provided when we clicked the PromoteSubscribers button, where we listed the Stages that would be affected by the promote. If we implement my suggestion in Issue 1, we should have enough space to present the same message we used to.

Signed-off-by: Remington Breeze <remington@breeze.software>
@rbreeze
Copy link
Contributor Author

rbreeze commented Sep 13, 2023

@jessesuen I've updated to address your comments, with some tweaks:

  1. Instead of a popup, I simply expand the Freight box to include both the contents and the confirmation
  2. Instead of writing which stages will be promoted, I highlight them in the DAG
CleanShot 2023-09-12 at 23 29 08@2x CleanShot 2023-09-12 at 23 28 57@2x CleanShot 2023-09-12 at 23 29 46@2x CleanShot 2023-09-12 at 23 30 03@2x

Copy link
Member

@jessesuen jessesuen left a comment

Choose a reason for hiding this comment

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

I think that should be good enough for now. LGTM!

Signed-off-by: Remington Breeze <remington@breeze.software>
@jessesuen jessesuen merged commit 704ff20 into main Sep 13, 2023
@jessesuen jessesuen deleted the promote-ui branch September 13, 2023 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants