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

fix(deps): use new inquirer #456

Merged
merged 27 commits into from
Jan 16, 2024
Merged

fix(deps): use new inquirer #456

merged 27 commits into from
Jan 16, 2024

Conversation

mshanemc
Copy link
Contributor

@mshanemc mshanemc commented Jan 8, 2024

requires salesforcecli/cli-plugins-testkit#610 for select automation

What does this PR do?

new inquirer via sf-plugins-core

other

  • test fixture for audit:messages instead of putting the plugin-generation command in the "unit test"
  • move command execution for audit:message into a nut and expand coverage
  • hide configure commands that aren't useful outside salesforce

What issues does this PR fix or reference?

@W-13801582@

@mshanemc mshanemc requested a review from a team as a code owner January 8, 2024 23:23
package.json Outdated Show resolved Hide resolved
src/generators/plugin.ts Outdated Show resolved Hide resolved
@mdonnalley
Copy link
Collaborator

QA

sf dev generate library
Npm scope prompt enforces @ prefix and defaults to @salesforce
⚠️ it doesn't mention anything in the initial prompt about needing to start with @ - nice to have??
❌ fails to proceed after submitting an answer

Time to build a library!
? Npm Scope @salesforce
✖ An error occured while running sf:library#prompting
Error (1): Cannot set properties of undefined (setting 'scope')

sf dev generate plugin
Are you building a plugin for an internal Salesforce team - Y

  • enter the name of your plugin enforces, valid npm package name plugin- prefix, and all lowercase chars
    • ⚠️ it enforces a valid npm package name but the validation error only mentions needing plugin- and lowercase chars
  • description prompt works
  • ✅ successfully generates plugin

Are you building a plugin for an internal Salesforce team - N

  • enter the name of your plugin enforces valid npm package name
  • description prompt works
  • author prompt works
  • code coverage selector works
  • ✅ successfully generates plugin

sf dev generate flag
command selector shows all existing commands
flag type selector shows all flag types

  • boolean prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required
  • custom prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
    • ❌ resulting custom flag is invalid. Needs to have a () at the end of Flags.custom, e.g. Flags.custom({summary: 'foo'})(),
  • directory prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple, exists prop
  • duration prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
    • ❌ It does not prompt for duration's unique props (like durationUnit)
  • file prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple, exists prop
  • integer prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple, min, max, default props
    • ❌ allows you to leave default unset even if you set a min or max
  • option prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
    • ❌ resulting option flag is invalid. Needs to have a () at the end of Flags.option, e.g. Flags.option({summary: 'foo'})(),
    • ⚠️ does this need to prompt for what the options should be? That way we can set options: ['foo', 'bar'] as const for them
  • optionalHub prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
  • optionalOrg prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
  • orgApiVersion prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
  • requiredHub prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
  • requiredOrg prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
  • salesforceId prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple, length (both, 15, 18, none), 3 char prefix (enforces length of 3)
  • string prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple
  • url prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char) required, multiple

@mdonnalley
Copy link
Collaborator

mdonnalley commented Jan 10, 2024

QA Round 2

sf dev generate library

⚠️ it doesn't mention anything in the initial prompt about needing to start with @ - nice to have??

🟢 Resolved

❌ fails to proceed after submitting an answer

🟢 Resolved

sf dev generate plugin

⚠️ it enforces a valid npm package name but the validation error only mentions needing plugin- and lowercase chars

Still a ⚠️ - non blocking though

sf dev generate flag

❌ resulting custom flag is invalid. Needs to have a () at the end of Flags.custom, e.g. Flags.custom({summary: 'foo'})(),

🟢 Resolved
options are not populated on new custom flag
⚠️ It seems odd to me to prompt for options on a custom flag since it's not required and there a ton of uses of the custom flag that don't involve options.

❌ It does not prompt for duration's unique props (like durationUnit)

🟢 Resolved

❌ allows you to leave default unset even if you set a min or max

🟢 Resolved

❌ resulting option flag is invalid. Needs to have a () at the end of Flags.option, e.g. Flags.option({summary: 'foo'})(),

🟢 Resolved
options are not populated on new custom flag

@mdonnalley
Copy link
Collaborator

QA Round 3

sf dev generate flag
✅ command selector shows all existing commands
✅ flag type selector shows all flag types
✅ boolean prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required
✅ custom prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple; resulting custom flag is valid.
✅ directory prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple, exists prop
✅ duration prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple, and custom props, default value is forced to between min and max
✅ file prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple, exists prop
✅ integer prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple, min, max, default props, default value is forced to between min and max
✅ option prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple, and options, resulting option flag is valid.

✅ all org flags prompt for "standard" props.

  • n sends user to typical prompts (name, summary, short char, required, multiple)
  • y sends user to name prompt
  • ⚠️ The standard definition prompts mentions (character, summary, description, default) and then the resulting flag doesn't have any of those props on it. This makes sense since Flags.optionalHub() has all those props set on it already but I'm wondering if it might be surprising to users - like maybe they'd be expecting defaults to populated for those properties?? Probably not a big deal - might just need slightly different messaging if it turns out to be confusing to people

✅ salesforceId prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple, length (both, 15, 18, none), 3 char prefix (enforces length of 3)
✅ string prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple
✅ url prompts for flag name (enforces uniqueness), summary (enforces capital letter and period), short char (validates single char), required, multiple

@mshanemc
Copy link
Contributor Author

  • ⚠️ The standard definition prompts mentions (character, summary, description, default) and then the resulting flag doesn't have any of those props on it. This makes sense since Flags.optionalHub() has all those props set on it already but I'm wondering if it might be surprising to users - like maybe they'd be expecting defaults to populated for those properties?? Probably not a big deal - might just need slightly different messaging if it turns out to be confusing to people

Yeah, I chose "don't put all those on there if they're not needed" vs. duplicating what's on the prebuilt flag. That's what our code usually does. That also gives the ability to pick up message improvements like https://github.com/salesforcecli/sf-plugins-core/pull/455/files (they wouldn't have gotten that if we had copied those into their flag)

@mdonnalley
Copy link
Collaborator

Yeah, I chose "don't put all those on there if they're not needed" vs. duplicating what's on the prebuilt flag. That's what our code usually does. That also gives the ability to pick up message improvements like https://github.com/salesforcecli/sf-plugins-core/pull/455/files (they wouldn't have gotten that if we had copied those into their flag)

To be clear, I wasn't advocating for adding the props when they're not needed - just that it might be surprising to developers who aren't familiar with how the flags work

@mdonnalley mdonnalley merged commit 6a674ee into main Jan 16, 2024
10 checks passed
@mdonnalley mdonnalley deleted the sm/new-inquirer branch January 16, 2024 19:44
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