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

feat: deprecate scopedEnvVarKey for scopedEnvVarKeys which accounts for binAliases #751

Merged
merged 3 commits into from
Jul 28, 2023

Conversation

WillieRuemmele
Copy link
Contributor

@W-13825184@

@@ -22,7 +22,7 @@ const pjson = {
files: [],
commands: {},
oclif: {
binAliases: ['foo', 'bar'],
binAliases: ['bar', 'baz'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the bin is foo, changed to remove confusing assertions

return v === '1' || v === 'true'
}

/**
* @deprecated - use scopedEnvVarKeys instead which will account for bin aliases, this could be private/removed in a major
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how I feel about deprecating this since the overwhelming majority of oclif CLIs won't ever use bin aliases. For them it would be annoying to have to deal with an array when it's always going to be an array with one element in it

I wonder if it might be better to keep it un-deprecated and just document clearly in the jsdoc that if they're using bin aliases they should use the other method

What do you think?

@mdonnalley mdonnalley merged commit 4787248 into main Jul 28, 2023
@mdonnalley mdonnalley deleted the wr/scopedEnvVars branch July 28, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants