-
Notifications
You must be signed in to change notification settings - Fork 94
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
fix(v2): Show error for unsaved data set rename #3328
Conversation
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v2-lts #3328 +/- ##
==========================================
+ Coverage 93.36% 93.38% +0.02%
==========================================
Files 105 105
Lines 11092 11117 +25
Branches 2426 2356 -70
==========================================
+ Hits 10356 10382 +26
+ Misses 735 734 -1
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Not sure why SonarCloud and the Pull Request Size bot is having a bad time. I think it might have been because I changed the base branch after opening the PR. That said, disregard the SonarCloud message as its not accurate. I've marked the SonarCloud message as outdated. |
Signed-off-by: Trae Yelovich <[email protected]>
📅 Suggested merge-by date: 12/3/2024 |
Signed-off-by: Trae Yelovich <[email protected]>
Quality Gate failedFailed conditions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made some comments on the changelog and other items.
|
||
expect(await TreeViewUtils.errorForUnsavedResource(ussNode)).toBe(true); | ||
expect(blockMocks.errorMessage).toHaveBeenCalledWith( | ||
"Unable to rename testFile.txt because you have unsaved changes in this file. " + "Please save your work and try again.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an extra space between the period and the quotation marks in the first message? Not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No extra spaces here - the text was simply split into two sections to avoid the "max length" limit that we have set for lines of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The +
sign seems unnecessary here since the text isn't split across multiple lines:
"Unable to rename testFile.txt because you have unsaved changes in this file. " + "Please save your work and try again.", | |
"Unable to rename testFile.txt because you have unsaved changes in this file. Please save your work and try again.", |
packages/zowe-explorer/__tests__/__unit__/utils/TreeViewUtils.unit.test.ts
Show resolved
Hide resolved
packages/zowe-explorer/__tests__/__unit__/utils/TreeViewUtils.unit.test.ts
Show resolved
Hide resolved
packages/zowe-explorer/__tests__/__unit__/utils/TreeViewUtils.unit.test.ts
Show resolved
Hide resolved
Signed-off-by: Trae Yelovich <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good stuff, @traeok, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't prevent me from renaming a member with unsaved changes. The V3 PR behaved as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once I fixed my environment, thanks @traeok and sorry for the false alarm
Proposed changes
v2 port of the error dialog from #3326
Release Notes
Milestone: 2.18.1
Changelog:
Types of changes
Checklist
General
yarn workspace vscode-extension-for-zowe vscode:prepublish
pnpm --filter vscode-extension-for-zowe vscode:prepublish
Code coverage
Deployment