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.

Sign up for GitHub

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

Jump to bottom

feat(ui): PR deep links in Promotion steps and in Promotion lists #3129

Merged
merged 6 commits into from
Dec 17, 2024
Merged

feat(ui): PR deep links in Promotion steps and in Promotion lists #3129

merged 6 commits into from
Dec 17, 2024

Conversation

Marvin9
Copy link
Contributor

@Marvin9 Marvin9 commented Dec 12, 2024

fixes #3024

Other than fix. This PR does some works for

  • Lays very soft foundation for future plugin architecture. UI takes opportunity of learn possible ways to split plugins. Note that this is not final and subject to change based on discussions when time comes but at the very least makes it easier to break into pieces
  • Removes the icons in promotion steps.. with deep links, it looked more busy with icons.
Screen.Recording.2024-12-12.at.4.42.55.PM.mov

Will cleanup some UI a bit but other than that its ready to review

@Marvin9
feat(ui): PR link in promotion step
6dad3a1 
Signed-off-by: Mayursinh Sarvaiya <[email protected]>
@Marvin9
feat: PR link in promotion list
3d763da 
Signed-off-by: Mayursinh Sarvaiya <[email protected]>
@Marvin9 Marvin9 added this to the v1.1.2 milestone Dec 12, 2024
@Marvin9 Marvin9 self-assigned this Dec 12, 2024
@Marvin9 Marvin9 requested a review from a team as a code owner December 12, 2024 20:50
@netlify Netlify
Copy link

netlify bot commented Dec 12, 2024 •
edited
Loading

✅ Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit a7fc349
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-io/deploys/6761e1ae264fe70008d4d1b4
😎 Deploy Preview https://deploy-preview-3129.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Marvin9 Marvin9 requested review from krancour and rpelczar December 12, 2024 20:52
@codecov Codecov
Copy link

codecov bot commented Dec 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.27%. Comparing base (a25b7da) to head (a7fc349).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3129      +/-   ##
==========================================
+ Coverage   51.26%   51.27%   +0.01%     
==========================================
  Files         283      285       +2     
  Lines       25563    25706     +143     
==========================================
+ Hits        13104    13182      +78     
- Misses      11762    11824      +62     
- Partials      697      700       +3

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

@Marvin9
chore(ui): fault tolerance of plugin
3adcf8b 
Signed-off-by: Mayursinh Sarvaiya <[email protected]>
rpelczar
rpelczar requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@rpelczar rpelczar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

IMO there is too much shared components, utils and types in plugins. Let's try to separate code as much as you can. Also, I'd like to have more clear structure.
Maybe:

-plugins

  • common
  • plugin1
  • plugin2
  • index.tsx

ui/src/features/stage/promotion-details-modal.tsx Outdated Show resolved Hide resolved
ui/src/features/stage/promotions.tsx Outdated Show resolved Hide resolved
@Marvin9
chore(ui): address review
c6da38e 
Signed-off-by: Mayursinh Sarvaiya <[email protected]>
@Marvin9 Marvin9 requested a review from rpelczar December 17, 2024 16:48
@Marvin9
Copy link
Contributor Author

Marvin9 commented Dec 17, 2024

IMO there is too much shared components, utils and types in plugins. Let's try to separate code as much as you can. Also, I'd like to have more clear structure. Maybe:

-plugins

  • common
  • plugin1
  • plugin2
  • index.tsx

Done! Thanks for this suggestion. I have use atoms instead of common as I think it explains as building blocks. But again this isn't final bullet proof and subject to change drastically in future

rpelczar
rpelczar approved these changes Dec 17, 2024
View reviewed changes
Copy link
Contributor

@rpelczar rpelczar left a comment

Choose a reason for hiding this comment

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

LGTM!

@Marvin9 Marvin9 force-pushed the Marvin9/feat-promotion-step-plugin branch from 8300806 to a7fc349 Compare December 17, 2024 20:40
@Marvin9 Marvin9 enabled auto-merge December 17, 2024 20:41
@Marvin9 Marvin9 added this pull request to the merge queue Dec 17, 2024
Merged via the queue into main with commit db17a2a Dec 17, 2024
17 checks passed
@Marvin9 Marvin9 deleted the Marvin9/feat-promotion-step-plugin branch December 17, 2024 21:05
diegocaspi pushed a commit to diegocaspi/kargo that referenced this pull request Dec 18, 2024
@Marvin9 @diegocaspi
feat(ui): PR deep links in Promotion steps and in Promotion lists (ak…
e4c3daa 
…uity#3129)

Signed-off-by: Mayursinh Sarvaiya <[email protected]>
Signed-off-by: Diego Caspi <[email protected]>
@fykaa fykaa mentioned this pull request Dec 19, 2024
Merged
fykaa pushed a commit to fykaa/kargo that referenced this pull request Dec 20, 2024
@Marvin9 @fykaa
feat(ui): PR deep links in Promotion steps and in Promotion lists (ak…
0cb8fec 
…uity#3129)

Signed-off-by: Mayursinh Sarvaiya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rpelczar rpelczar rpelczar approved these changes

@krancour krancour Awaiting requested review from krancour

Assignees

@Marvin9 Marvin9

Labels
area/ui kind/bug priority/high
Projects
None yet
Milestone
v1.1.2
Development

Successfully merging this pull request may close these issues.

PR link no longer displayed in the Promotion UI
2 participants
@Marvin9 @rpelczar