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

fix: show success notification when pushing multi-resource editor (#710) #800

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: show success notification when pushing multi-resource editor (#710) #800

wants to merge 1 commit into from

Conversation

adietish
Copy link
Collaborator

@adietish adietish commented Oct 14, 2024 •
edited
Loading

fixes #710

@adietish adietish self-assigned this Oct 14, 2024
@adietish adietish force-pushed the issue-710 branch 2 times, most recently from cfc24ae to 44440a8 Compare October 15, 2024 20:31
@adietish
Copy link
Collaborator Author

adietish commented Oct 16, 2024 •
edited
Loading

@sbouchet, @lgrossma: I'd love to get feedback on how this change feels for an end-user (a test-document with a deployment and a service is in the issue).

Does it feel right, odd? Is it possible for you to test it and tell me, please?

To me the cases where a single resource in a multi-resource document is modified and pushed is ok. More questionable is when you have multiple notifications are displayed:

ex.
the deployment has an error (ex replicas are "A", should be numerical) and the service was successfully updated.

@adietish adietish requested review from sbouchet and lgrossma October 17, 2024 12:47
@adietish
Copy link
Collaborator Author

@lgrossma: did you find the time to test this, please?

@adietish
Copy link
Collaborator Author

adietish commented Nov 7, 2024

@lgrossma reviewed this and thought that the way it behaves is ok.

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Nov 7, 2024

Quality Gate Passed Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbouchet sbouchet Awaiting requested review from sbouchet

@lgrossma lgrossma Awaiting requested review from lgrossma

At least 1 approving review is required to merge this pull request.

Assignees

@adietish adietish

Labels
do-not-merge/work-in-progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

As a user I want to know that my multi-resource editor successfully created the resource
1 participant
@adietish