-
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
Issue MVS Refinements #1230
Issue MVS Refinements #1230
Conversation
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
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.
Exactly what it says on the tin :) One grammar change ^^'
packages/zowe-explorer/i18n/sample/src/command/MvsCommandHandler.i18n.json
Outdated
Show resolved
Hide resolved
packages/zowe-explorer/i18n/sample/src/command/MvsCommandHandler.i18n.json
Outdated
Show resolved
Hide resolved
I agree with @katelynienaber's grammar nitpicking here. :P Aside from that, this PR looks good so far, but I will test more after changes are made. |
Signed-off-by: Billie Simmons <49491949+JillieBeanSim@users.noreply.github.com>
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 looks good to me! All unit and expected integration tests are passing. (See end of comment for more details.) In manual testing, the dialog text we currently have for issuing an MVS command all looks updated appropriately.
Thanks @JillieBeanSim for these refinements!
~
For integration tests, I got 3 failures. Two of them have also been on master
branch for a long time. The third was a timeout on Enter Pattern; should match data sets for multiple patterns
, which I started getting more recently when testing various PRs. However, I occasionally also see this happen on master
branch, so I wouldn't count it against this PR.
@JillieBeanSim Just curious as you are working on MVS commands - One of the two integration tests that consistently fails is Submit an MVS command
- Perhaps you might have an idea of how to get it passing again? I'm ok with this being fixed in a future PR, since it is a test that is also broken on master
branch, and I know you are continuing work on commands.
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.
Works great! Thank you @JillieBeanSim
Proposed changes
This addresses issues found in:
#875 & #802 -> update
Issue MVS Command
the dialog text#809 -> change the text in the Command Pallet from
Zowe: Issue TSO Command
which is linked to issue console command to readZowe: Issue MVS Command
Release Notes
Milestone: 1.13.1 or higher
Changelog: Issue MVS Command updates to Command Pallet and dialog of Text Boxes when prompted for command.
Types of changes
What types of changes does your code introduce to Zowe Explorer?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the revieweryarn workspace vscode-extension-for-zowe vscode:prepublish
has been executedFurther comments
I do notice a number of failing integration tests on Master branch that deal with USS now that weren't there recently.
3) Extension Integration Tests - USS
Enter USS Pattern
should output path that match the user-provided path
4) Extension Integration Tests - USS
Saving a USS File
should download, change, and re-upload a file:
5) ussNodeActions integration test
Rename USS File
should rename a uss file:
6) USSTree Integration Tests
Tests that getChildren returns valid list of elements:
7) ZoweUSSNode Integration Tests
Testing that getChildren returns the correct Thenable<ZoweUSSNode[]>: