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

Port zosconsole to next #3058

Merged
merged 33 commits into from
Sep 5, 2024
Merged

Port zosconsole to next #3058

merged 33 commits into from
Sep 5, 2024

Conversation

JillieBeanSim
Copy link
Contributor

@JillieBeanSim JillieBeanSim commented Aug 21, 2024

Proposed changes

Release Notes

Milestone: n/a port from main

Changelog: n/a

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds or improves functionality)
  • Breaking change (a change that would cause existing functionality to not work as expected)
  • Documentation (Markdown, README updates)
  • Other (please specify above in "Proposed changes" section)

Checklist

General

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • All PR dependencies have been merged and published (if applicable)
  • A GIF or screenshot is included in the PR for visual changes
  • The pre-publish command has been executed:
    • v2 and below: yarn workspace vscode-extension-for-zowe vscode:prepublish
    • v3: pnpm --filter vscode-extension-for-zowe vscode:prepublish

Code coverage

  • There is coverage for the code that I have added
  • I have added new test cases and they are passing
  • I have manually tested the changes

Deployment

  • I have added developer documentation (if applicable)
  • Documentation should be added to Zowe Docs
    • If you're an outside contributor, please post in the #zowe-doc Slack channel to coordinate documentation.
    • Otherwise, please check with the rest of the squad about any needed documentation before merging.
  • These changes may need ported to the appropriate branches (list here):

Further comments

crshnburn and others added 20 commits August 21, 2024 08:39
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Andrew Smithson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
…rofile list

Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Co-authored-by: Timothy Johnson <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
Signed-off-by: Billie Simmons <[email protected]>
@JillieBeanSim JillieBeanSim self-assigned this Aug 21, 2024
@JillieBeanSim JillieBeanSim changed the base branch from main to next August 21, 2024 13:59
Copy link

codecov bot commented Aug 21, 2024

Codecov Report

Attention: Patch coverage is 67.24138% with 19 lines in your changes missing coverage. Please review.

Project coverage is 92.73%. Comparing base (2a3d595) to head (91591c8).
Report is 34 commits behind head on next.

Files with missing lines Patch % Lines
...es/zowe-explorer/src/zosconsole/ZosConsolePanel.ts 62.74% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next    #3058      +/-   ##
==========================================
- Coverage   92.86%   92.73%   -0.13%     
==========================================
  Files         112      113       +1     
  Lines       11509    11566      +57     
  Branches     2560     2424     -136     
==========================================
+ Hits        10688    10726      +38     
- Misses        819      838      +19     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JillieBeanSim JillieBeanSim mentioned this pull request Aug 21, 2024
16 tasks
Copy link
Member

@traeok traeok 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 porting this over Billie!

I noticed that the "text field" for displaying the console output is smaller in the port:

image

The text field in v2 fills the remaining space:
image

@JillieBeanSim
Copy link
Contributor Author

Thanks for porting this over Billie!

I noticed that the "text field" for displaying the console output is smaller in the port:

image

The text field in v2 fills the remaining space: image

Hey @traeok this happened when the changes to HTMLTemplate were reverted per @zFernand0 's request. Do we change it back for this to work as it does in v2 or will that change affect other new v3 functionality that may use the HTMLTemplate?

@zFernand0
Copy link
Member

Hey traeok this happened when the changes to HTMLTemplate were reverted per zFernand0 's request. Do we change it back for this to work as it does in v2 or will that change affect other new v3 functionality that may use the HTMLTemplate?

Hey @JillieBeanSim,
Sorry for the confusion. If we want to port the same console terminal (with a webview implementation) we will need to merge the two (main and next) HTML Templates without impacting any existing functionality.

I don't mind opening a PR with the integrated terminal implementation before the end of the week. That way we don't have to merge the templates together and then refactor the implementation to use the built-in VSCode capabilities 😋

@zFernand0
Copy link
Member

Hey,
I wanted to summarize the discussion that we had during standup today (for transparency)

We are fast approaching the Breaking-change code-freeze date for V3.

We mentioned that it might be a bit late to introduce more enhancements into our version 3.0.0 since the GA date is a little over month away.

We also discussed the potential breaking change (for both, users[UX] and developers[webview]) if we decide to implement integrated terminals after having a webview implementation in V3.


Please feel free to expand if I missed anything 😋

@adam-wolfe
Copy link
Contributor

Hey, I wanted to summarize the discussion that we had during standup today (for transparency)

We are fast approaching the Breaking-change code-freeze date for V3.

We mentioned that it might be a bit late to introduce more enhancements into our version 3.0.0 since the GA date is a little over month away.

We also discussed the potential breaking change (for both, users[UX] and developers[webview]) if we decide to implement integrated terminals after having a webview implementation in V3.

Please feel free to expand if I missed anything 😋

This is a good summary, my questions are:

  1. If we switch from a WebView (3.0.0) to an integrated terminal (3.1.0), would this actually break something (other than being a change to UX)?
    a. Could we make the WebView implementation 'deprecated' somehow in V3 to avoid having extenders try to do anything with it?
  2. Would it be safer to release 3.0.0 without the z/OS console at all (and then introduce the integrated terminal in 3.1.0)?

If I had to choose a way forward, I would stick with the WebView implementation for v3.0.0, then target the integrated MVS+TSO+SSH terminal work for V3.1.0 (code freeze on Jan 02, 2025).

@JillieBeanSim
Copy link
Contributor Author

do we want to mark it deprecated for v3 too moving forward for awareness with plan to move over to the integrated terminal?

Comment on lines 30 to 35
{{#uris.resource.css}}
<link type="text/css" rel="stylesheet" href="{{ uris.resource.css }}" />
{{/uris.resource.css}}
{{#uris.resource.codicons}}
<link type="text/css" rel="stylesheet" href="{{ uris.resource.codicons }}" />
{{/uris.resource.codicons}}
Copy link
Member

@traeok traeok Aug 30, 2024

Choose a reason for hiding this comment

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

This logic is required to support our webviews in v3 - we should add these back to avoid breaking other webviews

adam-wolfe
adam-wolfe previously approved these changes Sep 4, 2024
Copy link
Contributor

@adam-wolfe adam-wolfe left a comment

Choose a reason for hiding this comment

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

LGTM

zFernand0
zFernand0 previously approved these changes Sep 5, 2024
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

LGTM! 😋

Note that the console/terminal will not work unless the session/profile is loaded in a tree and has been validated.
image

Copy link
Member

@traeok traeok left a comment

Choose a reason for hiding this comment

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

I pushed up a change to fix a couple missing items from the HTML template.
Tested and LGTM - thanks again @JillieBeanSim for the port and for addressing feedback

Copy link

sonarcloud bot commented Sep 5, 2024

@zFernand0 zFernand0 merged commit 7231bad into next Sep 5, 2024
18 of 20 checks passed
@zFernand0 zFernand0 deleted the port/zosconsole branch September 5, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

6 participants