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

docs: preview command #1384

Merged
merged 11 commits into from
Jan 23, 2024
Merged

docs: preview command #1384

merged 11 commits into from
Jan 23, 2024

Conversation

volodymyr-rutskyi
Copy link
Contributor

What/Why/How?

Docs for Preview project command.

Reference

Implementation PR: #1367

Testing

Screenshots (optional)

Check yourself

  • Code is linted
  • Tested with redoc/reference-docs/workflows (internal)
  • All new/updated code is covered with tests

Security

  • Security impact of change has been considered
  • Code follows company security practices and guidelines

@volodymyr-rutskyi volodymyr-rutskyi requested a review from a team as a code owner January 8, 2024 14:13
Copy link

changeset-bot bot commented Jan 8, 2024

🦋 Changeset detected

Latest commit: 8080fb9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@redocly/cli Minor
@redocly/openapi-core Minor

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

Copy link
Contributor

github-actions bot commented Jan 8, 2024

Command Mean [s] Min [s] Max [s] Relative
redocly lint packages/core/src/benchmark/benches/rebilly.yaml 1.092 ± 0.030 1.044 1.156 1.00
redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml 1.098 ± 0.021 1.063 1.129 1.00 ± 0.03

Copy link
Contributor

github-actions bot commented Jan 8, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 75.8% 4100/5409
🟡 Branches 66.07% 2183/3304
🟡 Functions 68.14% 663/973
🟡 Lines 76% 3847/5062

Test suite run success

671 tests passing in 94 suites.

Report generated by 🧪jest coverage report action from 8080fb9

Copy link
Contributor

@lornajane lornajane left a 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!

@@ -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.
Copy link
Contributor

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 Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved

### 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.
Copy link
Contributor

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.

docs/commands/preview.md Outdated Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved
volodymyr-rutskyi and others added 2 commits January 10, 2024 17:29
@volodymyr-rutskyi volodymyr-rutskyi requested a review from a team as a code owner January 11, 2024 07:36
Copy link
Contributor

@lornajane lornajane left a 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.

docs/commands/preview.md Outdated Show resolved Hide resolved
docs/commands/preview.md Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved
docs/commands/preview.md Outdated Show resolved Hide resolved
Co-authored-by: Lorna Jane Mitchell <github@lornajane.net>
@lornajane
Copy link
Contributor

I'm not sure why Vale is returning as neutral, it has annotated a problem

Copy link
Contributor

@lornajane lornajane left a 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!

Copy link
Member

@adamaltman adamaltman left a 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.

docs/commands/index.md Outdated Show resolved Hide resolved
Copy link
Member

@adamaltman adamaltman left a 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?

Copy link
Contributor

@lornajane lornajane left a 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.

@tatomyr tatomyr merged commit 4ff9eb6 into main Jan 23, 2024
30 checks passed
@tatomyr tatomyr deleted the docs/preview-project-command branch January 23, 2024 10:49
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.

4 participants