Skip to content
This repository has been archived by the owner on Nov 13, 2023. It is now read-only.

Fix plugin install error not displayed correctly #958

Merged
merged 7 commits into from
Mar 23, 2023
Merged

Fix plugin install error not displayed correctly #958

merged 7 commits into from
Mar 23, 2023

Conversation

zFernand0
Copy link
Member

@t1m0thyj @zFernand0
Replace spawnSync with wrapper method that handles errors
4a6d868 
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
@t1m0thyj @zFernand0
Update unit tests for plugin install failure
7e45bf6 
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
@t1m0thyj @zFernand0
Remove unnecessary logging of stderr
fc8ac7e 
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
@t1m0thyj @zFernand0
Add tests to increase NpmFunctions coverage
898b135 
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
@zFernand0 zFernand0 requested a review from t1m0thyj March 20, 2023 19:09
@zFernand0 zFernand0 changed the base branch from master to zowe-v1-lts March 20, 2023 19:10
@zFernand0 zFernand0 removed the request for review from t1m0thyj March 20, 2023 19:27
@zFernand0
fix lint and audit warnings
1e9246e 
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Mar 20, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.17 🎉

Comparison is base (199f048) 82.72% compared to head (4759498) 82.90%.

Additional details and impacted files 
@@               Coverage Diff               @@
##           zowe-v1-lts     #958      +/-   ##
===============================================
+ Coverage        82.72%   82.90%   +0.17%     
===============================================
  Files              163      163              
  Lines             7953     7948       -5     
  Branches          1458     1459       +1     
===============================================
+ Hits              6579     6589      +10     
+ Misses            1370     1355      -15     
  Partials             4        4
Impacted Files Coverage Δ
packages/console/src/Console.ts 88.14% <100.00%> (ø)
...s/imperative/src/plugins/utilities/NpmFunctions.ts 100.00% <100.00%> (+38.46%) ⬆️
...e/src/plugins/utilities/npm-interface/uninstall.ts 96.96% <100.00%> (-0.09%) ⬇️
...erative/src/plugins/utilities/runValidatePlugin.ts 100.00% <100.00%> (ø)
packages/logger/src/Logger.ts 83.56% <100.00%> (ø)
packages/utilities/src/ProcessUtils.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@zFernand0 zFernand0 self-assigned this Mar 21, 2023
@zFernand0
fix unit test
78f2f9f 
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
@zFernand0 zFernand0 marked this pull request as ready for review March 22, 2023 12:49
t1m0thyj
t1m0thyj reviewed Mar 22, 2023
View reviewed changes
Copy link
Member

@t1m0thyj t1m0thyj left a comment

Choose a reason for hiding this comment

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

Left a comment about a number of methods that were added to ProcessUtils - not sure if we want to add any more methods than necessary, since we shouldn't be making enhancements to v1-lts.

Also, I know @awharn has identified another issue with plugin install - not sure if we want to fix that in this PR or a separate one 🙂

packages/utilities/src/ProcessUtils.ts Outdated Show resolved Hide resolved
@zFernand0
remove enhancements from v1-lts
4759498 
Signed-off-by: zFernand0 <37381190+zFernand0@users.noreply.github.com>
@zFernand0 zFernand0 requested a review from traeok March 22, 2023 18:34
traeok
traeok approved these changes Mar 22, 2023
View reviewed changes
packages/console/src/Console.ts
import { TextUtils } from "../../utilities";
import { TextUtils } from "../../utilities/src/TextUtils";
Copy link
Member

Choose a reason for hiding this comment

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

Did we remove TextUtils exports from utilities/index.ts? Looks like we're still doing an export * from "TextUtils"

packages/logger/src/Logger.ts
import { TextUtils } from "../../utilities";
import { TextUtils } from "../../utilities/src/TextUtils";
Copy link
Member

Choose a reason for hiding this comment

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

Same as previous comment (completely fine with letting this as-is, just something I noticed during review)

@sonarcloud
Copy link

sonarcloud bot commented Mar 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@zFernand0 zFernand0 merged commit f357f31 into zowe-v1-lts Mar 23, 2023
@zFernand0 zFernand0 deleted the port-955 branch March 23, 2023 11:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@t1m0thyj t1m0thyj t1m0thyj approved these changes

@traeok traeok traeok approved these changes

@awharn awharn Awaiting requested review from awharn

Assignees

@zFernand0 zFernand0

Labels
release-current
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zFernand0 @t1m0thyj @traeok