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

Removed v1 changes from vsce package #185

Merged
merged 4 commits into from
Jan 6, 2025

Conversation

almightyrush
Copy link
Contributor

@almightyrush almightyrush commented Dec 23, 2024

What It Does

Removed the V1 changes which are no longer used.
How to Test

Review Checklist
I certify that I have:

Additional Comments

@codecov-commenter
Copy link

codecov-commenter commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.76%. Comparing base (7ed9a5b) to head (1a3d199).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #185   +/-   ##
=======================================
  Coverage   93.76%   93.76%           
=======================================
  Files          75       75           
  Lines         770      770           
  Branches       43       43           
=======================================
  Hits          722      722           
  Misses         48       48           

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

@almightyrush almightyrush linked an issue Dec 23, 2024 that may be closed by this pull request
Copy link
Contributor

@chinmdas chinmdas 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..!!!

Signed-off-by: Rushabh Sojitra <[email protected]>
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.

Changes LGTM, thanks @almightyrush! Will re-approve if needed once conflicts are resolved.


I noticed that the quick pick for adding a CICS profile has a strange placeholder. Minor cosmetic bug so not requesting changes here.

CICS:
image

Zowe Explorer Data Sets (for reference):
image

@davenice
Copy link
Contributor

davenice commented Jan 6, 2025

@traeok - when do you see that placeholder? i don't see it 🤔

@traeok
Copy link
Member

traeok commented Jan 6, 2025

@traeok - when do you see that placeholder? i don't see it 🤔

This is what shows when clicking on the "+" icon (Create a CICS Profile) in the CICS tree view:
image

Looks like it's coming from line 107 of CICSTree.ts, but these changes were made outside of this PR, so it can be resolved separately 😋

@almightyrush almightyrush requested a review from traeok January 6, 2025 16:44
@almightyrush
Copy link
Contributor Author

@traeok - Resolved conflicts

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.

Changes LGTM! 🙏


Thanks for keeping the discussion here in the PR! 🙏

@davenice
Copy link
Contributor

davenice commented Jan 6, 2025

@traeok - when do you see that placeholder? i don't see it 🤔
Looks like it's coming from line 107 of CICSTree.ts, but these changes were made outside of this PR, so it can be resolved separately 😋

Oh - the icon? I thought you meant demo.cics Yes! I guess that's the default? Good spot :-) @chinmdas has this fixed in his next PR i think 👌

@zFernand0 zFernand0 merged commit 8bd2870 into zowe:main Jan 6, 2025
16 checks passed
@zFernand0 zFernand0 added the release-current Indicates that there is no new functionality being delivered label Jan 6, 2025
@almightyrush almightyrush deleted the removed-v1-changes branch January 8, 2025 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-current Indicates that there is no new functionality being delivered
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

Remove v1 deprecated code from the extension
6 participants