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

vdk-jupyter: add run result to the deployment result dialog #2605

Merged

Conversation

duyguHsnHsn
Copy link
Collaborator

What: Added run result message to the deployment result dialog ( if run before deployment is selected)
Current dialog:
Screenshot 2023-08-25 at 10 36 53
Screenshot 2023-08-25 at 10 37 01

Why: It used to not contain the successful run message which left the suers with confusion around whether really the run was successful

Signed-off-by: Duygu Hasan hduygu@vmware.com

@duyguHsnHsn duyguHsnHsn marked this pull request as ready for review August 25, 2023 07:47
Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Looks good to me . I added some non-blocking-a-merge suggestions for improvements.

@antoniivanov
Copy link
Collaborator

image

The text here looks badly formated.
It would have been better to have spaces or new line between "sent suffully!New .."

Copy link
Contributor

@yonitoo yonitoo left a comment

Choose a reason for hiding this comment

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

Overall, LGTM! I left a few minor suggestions. I agree with the comment by @antoniivanov about the formatting, tho. A space after "successfully!" would work, although imo newline would be better.

@duyguHsnHsn duyguHsnHsn merged commit f4db01d into main Aug 25, 2023
10 of 11 checks passed
@duyguHsnHsn duyguHsnHsn deleted the person/hduygu/vdk-jupyter-deployment-run-result-message branch August 25, 2023 11:16
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.

None yet

4 participants