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

Add ErrorHandling for OpenWithEncoding function #3438

Merged
merged 3 commits into from
Feb 11, 2025

Conversation

likhithanimma1
Copy link
Contributor

Proposed changes

Added try catch block for openWithEncoding function in order to catch any of the errors thrown.

Release Notes

Milestone:

Changelog:

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

Signed-off-by: likhithanimma1 <[email protected]>
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.24%. Comparing base (51d6ab4) to head (c91ec78).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #3438    +/-   ##
========================================
  Coverage   93.23%   93.24%            
========================================
  Files         120      120            
  Lines       12662    12664     +2     
  Branches     2922     2755   -167     
========================================
+ Hits        11806    11808     +2     
  Misses        855      855            
  Partials        1        1            

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

@likhithanimma1 likhithanimma1 added the no-changelog Add to PR's that don't require a CHANGELOG update label Feb 6, 2025
Copy link

github-actions bot commented Feb 6, 2025

📅 Suggested merge-by date: 2/20/2025

Signed-off-by: likhithanimma1 <[email protected]>
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! 😋

I do not disagree with the changes, but I'm a bit curious how this can be properly tested. 😕
Right now, there is no way to use the Open With Encoding via the command palette since it doesn't work without a node. 🤔

I could be missing something but if credentials are invalid, I don't believe you will be able to get to the spoolfile to right click it and open it with a specific encoding.

That said, I still like the changes in preparation for when we make the openWithEncoding command work from the command palette 😋

@likhithanimma1
Copy link
Contributor Author

LGTM! 😋

I do not disagree with the changes, but I'm a bit curious how this can be properly tested. 😕 Right now, there is no way to use the Open With Encoding via the command palette since it doesn't work without a node. 🤔

I could be missing something but if credentials are invalid, I don't believe you will be able to get to the spoolfile to right click it and open it with a specific encoding.

That said, I still like the changes in preparation for when we make the openWithEncoding command work from the command palette 😋

We have function fetchSpoolAtUri where we are using it inside the OpenWithEncoding function which may throw an error. So, tried to handle it with try catch.

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.

LGTM, thanks @likhithanimma1

@JillieBeanSim JillieBeanSim merged commit 1027fe3 into main Feb 11, 2025
24 checks passed
@JillieBeanSim JillieBeanSim deleted the openWithEncoding/errorHandling branch February 11, 2025 13:32
likhithanimma1 added a commit that referenced this pull request Feb 17, 2025
* Add ErrorHandling

Signed-off-by: likhithanimma1 <[email protected]>

* Increase unit test coverage

Signed-off-by: likhithanimma1 <[email protected]>

---------

Signed-off-by: likhithanimma1 <[email protected]>
Co-authored-by: Fernando Rijo Cedeno <[email protected]>
@likhithanimma1 likhithanimma1 mentioned this pull request Feb 17, 2025
15 tasks
zFernand0 added a commit that referenced this pull request Feb 18, 2025
* Add ErrorHandling



* Increase unit test coverage



---------

Signed-off-by: likhithanimma1 <[email protected]>
Co-authored-by: Fernando Rijo Cedeno <[email protected]>
@JillieBeanSim JillieBeanSim added this to the v3.1.2 milestone Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Add to PR's that don't require a CHANGELOG update size/S
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

5 participants