-
Notifications
You must be signed in to change notification settings - Fork 43
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(openapi): add --dryRun
option
#612
feat(openapi): add --dryRun
option
#612
Conversation
@kanadgupta, what do you think we should do in the case where a user does a we automatically return this notification letting them know that we're attempting to upload it. should we change the copy at all to avoid confusion or is that okay? |
@darrenyong good question... I think it's okay to leave the copy as is! I think it's clear from these logs what's happening:
I think we should clean up the dry run log copy a bit but I'll leave a suggestion during my actual review! |
@kanadgupta merged main 😎 |
--dryRun
option
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.
A few copy edit suggestions below but the code and tests look great! Could you also add a little note about the dry run flag in the README.md
file? I think it makes sense for that note to live around 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.
Super tiny docs suggestion but otherwise looks LGTM 🚀
(also ticketed the docs change in RM-5475!)
🧰 Changes
This adds a
--dryRun
option to theopenapi
command so users can debug their spec creation and updating.🧬 QA & Testing
Provide as much information as you can on how to test what you've done.