-
Notifications
You must be signed in to change notification settings - Fork 87
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
Remove use of deprecated utils functions & Enable deprecation linter #2169
Conversation
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
… abort Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
packages/zosfiles/__tests__/__unit__/methods/utilities/Utilities.unit.test.ts
Fixed
Show fixed
Hide fixed
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Timothy Johnson <timothy.johnson@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.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.
Looks pretty good, thanks @awharn! Left a few comments
@@ -70,7 +70,7 @@ export default class ApimlAutoInitHandler extends BaseAutoInitHandler { | |||
*/ | |||
protected async doAutoInit(session: AbstractSession, params: IHandlerParameters): Promise<IConfig> { | |||
// Login with token authentication first, so we can support certificates | |||
if ((session.ISession.user && session.ISession.password) || (session.ISession.cert && session.ISession.certKey)) { | |||
if (session.ISession.user && session.ISession.password || session.ISession.cert && session.ISession.certKey) { |
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.
Generally I like the no-extra-parens
rule, but in a few cases like this one it may make the code less readable. I'm probably just dumb but it's not obvious to me what the order of precedence is across these 4 conditions. Not a request to change anything, just an observation 😋
packages/imperative/src/imperative/__tests__/plugins/PluginRequireProvider.unit.test.ts
Outdated
Show resolved
Hide resolved
const initPath = ImperativeConfig.instance.callerLocation ?? require.main.filename; | ||
if (initPath != null) { | ||
requireOpts.paths = [initPath, ...require.resolve.paths("@zowe/secrets-for-zowe-sdk")]; |
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.
We may want to keep the code structured the same as before:
const initPath = ImperativeConfig.instance.callerLocation ?? require.main.filename; | |
if (initPath != null) { | |
requireOpts.paths = [initPath, ...require.resolve.paths("@zowe/secrets-for-zowe-sdk")]; | |
if (require.main?.filename != null) { | |
requireOpts.paths = [require.main.filename, ...require.resolve.paths("@zowe/secrets-for-zowe-sdk")]; |
I don't think we want to add a reference to ImperativeConfig.instance
here because it is called by extenders like Zowe Explorer outside of a CLI context where the Imperative config is undefined.
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.
I was under the impression that was the reason why it falls back to require.main.filename, since if there is no callerLocation, then it just behaves as it did before.
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.
Follow up to our earlier discussion - ImperativeConfig.instance.callerLocation
returns null
so it would be ok to access in Zowe Explorer. However this may only be because we call ProfileInfo.initImpUtils
which initializes a fake Imperative config object. Thanks for removing the reference, I think it is the safer option for extenders using our SDKs - will leave this comment unresolved for now to get a second opinion 😋
const maxWidth = !((yargs.terminalWidth() && yargs.terminalWidth() > 0) == null) ? | ||
yargs.terminalWidth() - widthSafeGuard : preferredWidth; |
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 faithfully reflects the old logic, but I think the old logic may have been wrong - would the following make more sense?
const maxWidth = !((yargs.terminalWidth() && yargs.terminalWidth() > 0) == null) ? | |
yargs.terminalWidth() - widthSafeGuard : preferredWidth; | |
const maxWidth = (!(yargs.terminalWidth() == null) && yargs.terminalWidth() > 0) ? | |
yargs.terminalWidth() - widthSafeGuard : preferredWidth; |
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
…nter Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.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.
LGTM, thanks @awharn!
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.
LGTM, thanks for all your work on this Andrew! I left a couple comments but I don't have any requests for changes.
packages/cli/src/provisioning/list/registry/RegistryInstances.handler.ts
Outdated
Show resolved
Hide resolved
packages/imperative/src/imperative/__tests__/plugins/PluginRequireProvider.unit.test.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
…nter Signed-off-by: Andrew W. Harn <andrew.harn@broadcom.com>
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Release succeeded for the The following packages have been published:
Powered by Octorelease 🚀 |
What It Does
Stops using deprecated utility functions
Changes logic to use
require.main
overprocess.mainModule
, which is deprecatedEnables linters for detecting extra parenthesis and deprecated function use
How to Test
Review Checklist
I certify that I have:
Additional Comments