-
Notifications
You must be signed in to change notification settings - Fork 183
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
PDE-3732 feat(cli): Implement individual field flags for register command #618
Conversation
allowNo: true, | ||
}), | ||
}, | ||
}); | ||
RegisterCommand.examples = [ | ||
'zapier register', | ||
'zapier register "My Cool Integration"', |
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.
Can we add some more examples here?
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.
Remove non-functional "Learn more" text from homepage URL flag description Co-authored-by: Kola Erinoso <kola-er@users.noreply.github.com>
…egister.js Co-authored-by: Chang-Hung Liang <chang-hung.liang@zapier.com>
…ds/register.js Co-authored-by: Chang-Hung Liang <chang-hung.liang@zapier.com>
…f/hooks/getAppRegistrationFieldChoices.js Co-authored-by: Chang-Hung Liang <chang-hung.liang@zapier.com>
This enables us to see all available options when running `zapier register --help`
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 added a few more suggestions in the review. To summarize what we've discussed, I think this is ready to merge once these are also addressed:
- write unit tests for the
register
command- use
nock
to mockfields-choices
endpoint - remove test-related code from
register.js
- remove smoke tests
- use
- add the 140-char limit check for the
--description
flag - add some valid examples for the
register
command - add
?formId=create
in the API call sosubscription
gets saved (related Slack conversation)
tests * Move nock from devDependencies to dependencies in CLI package.json * Add chai to devDependencies (requirement of @oclif/test) * Remove mock/testing code from getAppRegistrationFieldChoices hook
* This commit refactors the prompt validation to allow for multiple custom validators * This commit refactors the RegisterCommand tests to be more uniform with the BaseCommand test
…hen validating enum fields Co-authored-by: Chang-Hung Liang <chang-hung.liang@zapier.com>
* This commit also utilizes nock for API requests in "zapier register should enforce character limits on flags" test cases
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 made two additional changes in the test code:
- hide stdout/stderr
- replace
fs.existsSync
withfs.statSync
Now this looks really good. Nice work, thanks!
@eliangcs Nice! I was struggling to hide stdout/stderr for some reason on my local, so I'm glad you updated that. I'm going to merge this PR now. |
This PR adds prompts for required information when running
zapier register
. It also allows users to pass these values in via flags.