-
Notifications
You must be signed in to change notification settings - Fork 155
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
docs: preview command #1384
docs: preview command #1384
Conversation
🦋 Changeset detectedLatest commit: 8080fb9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Coverage report
Test suite run success671 tests passing in 94 suites. Report generated by 🧪jest coverage report action from 8080fb9 |
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.
Thank you for great documentation! I made a bunch of suggestions and comments, but they're all minor. Great job!
docs/commands/index.md
Outdated
@@ -8,6 +8,7 @@ Documentation commands: | |||
|
|||
- [`preview-docs`](preview-docs.md) Preview API reference docs for the specified API description. | |||
- [`build-docs`](build-docs.md) Build API description into an HTML file. | |||
- [`preview`](preview.md) Preview Redocly project. |
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 gave the same feedback on the command itself, can we expand on what's happening here? Not all projects can be previewed here, and we should imply that we're starting a preview server locally, so the user knows what to expect
docs/commands/preview.md
Outdated
|
||
### Select a product for preview | ||
|
||
Instead of inferring the product package to use for preview from `package.json` or using Realm by default, you can specify the product package as an argument. |
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.
To make the wording more positive, start with "Specify the package to use ...." and then explain that if the argument isn't supplied, we'll try to guess.
Co-authored-by: Lorna Jane Mitchell <github@lornajane.net>
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.
There was a Vale problem and I had a couple of suggestions on the new content.
Co-authored-by: Lorna Jane Mitchell <github@lornajane.net>
I'm not sure why Vale is returning as neutral, it has annotated a problem |
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 is a great resource, thank you!
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 is a blocker for me.
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.
Is there also a missing changelog about adding this command?
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.
Changeset looks good, let's address Adam's comments and I think we are ready.
Co-authored-by: Adam Altman <adam@redocly.com>
What/Why/How?
Docs for Preview project command.
Reference
Implementation PR: #1367
Testing
Screenshots (optional)
Check yourself
Security