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: show history about completed runs in each cron workflow #11811

Merged
merged 4 commits into from
Sep 21, 2023
Merged

feat: show history about completed runs in each cron workflow #11811

merged 4 commits into from
Sep 21, 2023

Conversation

juijeong8324
Copy link
Contributor

Fixes #11706

Motivation

I want to check history about completed runs in each cron workflows.

Modifications

image
You can check the history in cron workflow details page when you click specific cron-workflow.

image
Also, If the cron workflow doesn't have completed workflows, you can see explanation as above.

Verification

Local running

@juijeong8324 @yunwoo-yu
feat: show history about completed runs in each cron workflow
83826ee 
Co-authored-by: yunwoo-yu <skypnal12@gmail.com>
Signed-off-by: juijeong8324 <juijeong8324@gmail.com>
@agilgur5 agilgur5 self-requested a review September 13, 2023 15:42
@agilgur5 agilgur5 mentioned this pull request Sep 13, 2023
Closed
@agilgur5
Copy link
Member

agilgur5 commented Sep 13, 2023 •
edited
Loading

Follow-up to my review of #11790

@juijeong8324
Copy link
Contributor Author

juijeong8324 commented Sep 13, 2023 •
edited by agilgur5
Loading

Follow-up to my review of #11790

Actually I already follow-up your review of #11790! If you have some advice, please let me know😀

@agilgur5
Copy link
Member

agilgur5 commented Sep 13, 2023 •
edited
Loading

Actually I already follow-up your review of #11789! If you have some advice, please let me know😀

I meant that this new PR is the follow-up (it addresses my review)

agilgur5
agilgur5 reviewed Sep 13, 2023
View reviewed changes
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Great work iterating on this!

There's 1 potential issue with the CSS, otherwise comments below are mostly stylistic.

ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.scss Show resolved Hide resolved
ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx Outdated Show resolved Hide resolved
ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx Show resolved Hide resolved
ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx Outdated Show resolved Hide resolved
@juijeong8324 juijeong8324 force-pushed the juijeong8324-cron-workflow branch 2 times, most recently from 2948e98 to 83826ee Compare September 14, 2023 16:50
@juijeong8324 @yunwoo-yu
style: Modify css selector and code
a9be6e1 
Co-authored-by: yunwoo-yu <skypnal12@gmail.com>
Signed-off-by: juijeong8324 <juijeong8324@gmail.com>
@juijeong8324 @yunwoo-yu
style: Modify cron-workflow-details.scss using descendant combinator
3657359 
Co-authored-by: yunwoo-yu <skypnal12@gmail.com>
Signed-off-by: juijeong8324 <juijeong8324@gmail.com>
@agilgur5
Copy link
Member

Tested locally and confirmed that this is working and not affecting other lists!

Cron Workflow page:
Screenshot 2023-09-20 at 11 23 29 AM

Workflow List page working as before:
Screenshot 2023-09-20 at 11 23 47 AM

agilgur5
agilgur5 reviewed Sep 20, 2023
View reviewed changes
Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Thanks for the great work @juijeong8324 and @yunwoo-yu !

There's one tiny lint error that needs fixing, otherwise LGTM 🙂

ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx Outdated Show resolved Hide resolved
terrytangyuan
terrytangyuan approved these changes Sep 21, 2023
View reviewed changes
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Pending CI build fix

@juijeong8324 @yunwoo-yu
fix: Sort imports for lint error
907bc6a 
Co-authored-by: yunwoo-yu <skypnal12@gmail.com>
Signed-off-by: juijeong8324 <juijeong8324@gmail.com>
@terrytangyuan terrytangyuan enabled auto-merge (squash) September 21, 2023 14:49
@terrytangyuan terrytangyuan merged commit 5def528 into argoproj:master Sep 21, 2023
25 checks passed
@agilgur5 agilgur5 mentioned this pull request Sep 22, 2023
Closed
8 tasks
@juijeong8324
Copy link
Contributor Author

@agilgur5 Thank you so much for your help! 🥰 I was able to solve and contribute to a UI issue for the first time.😎
I will try to contribute a lot to Argo-workflows!!

@agilgur5
Copy link
Member

Happy to help @juijeong8324! ❤️ Congrats on contributing a new UI feature for the first time! 🎉

If you (or @yunwoo-yu or any other folks y'all collaborate with) are interested in UI work in particular, we have a project with many issues open 😄 😉
Feel free to tag me in any issues you have questions about or for code review on any PRs 🙂

@juijeong8324 juijeong8324 deleted the juijeong8324-cron-workflow branch September 30, 2023 04:29
@agilgur5 agilgur5 mentioned this pull request Oct 20, 2023
Closed
3 tasks
This was referenced Feb 16, 2024
Closed
Open
@agilgur5 agilgur5 mentioned this pull request Feb 21, 2024
Closed
@agilgur5 agilgur5 mentioned this pull request Jun 30, 2024
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agilgur5 agilgur5 agilgur5 left review comments

@yunwoo-yu yunwoo-yu yunwoo-yu left review comments

@terrytangyuan terrytangyuan terrytangyuan approved these changes

Assignees
No one assigned
Labels
area/cron-workflows area/ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UI - show history about completed runs in each cron workflow
4 participants
@juijeong8324 @agilgur5 @terrytangyuan @yunwoo-yu