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

feat: add the eject and translate commands #1668

Merged
merged 10 commits into from
Aug 23, 2024

Conversation

tatomyr
Copy link
Contributor

@tatomyr tatomyr commented Aug 16, 2024

What/Why/How?

Added the translate and eject commands.

Also, refactored types.

Open questions: https://github.com/Redocly/redocly/issues/9612#issuecomment-2298159934 .

Another question: do we want to use options in camel case like --configDir? We mostly use kebab-case for options (except for --outDir in the split command and some options in the build-docs command which is legacy of redoc-cli I believe).

Reference

Resolves #1629

The eject docs PR: https://github.com/Redocly/redocly/pull/10344

Testing

Screenshots (optional)

Check yourself

  • Code changed? - Tested with redoc/reference-docs/workflows (internal)
  • All new/updated code is covered with tests
  • New package installed? - Tested in different environments (browser/node)

Security

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

@tatomyr tatomyr self-assigned this Aug 16, 2024
Copy link

changeset-bot bot commented Aug 16, 2024

🦋 Changeset detected

Latest commit: 46595da

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 Aug 16, 2024

Command Mean [ms] Min [ms] Max [ms] Relative
redocly lint packages/core/src/benchmark/benches/rebilly.yaml 999.0 ± 22.6 958.9 1035.3 1.02 ± 0.03
redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml 983.1 ± 18.0 956.9 1015.2 1.00

Copy link
Contributor

github-actions bot commented Aug 16, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 77.5% 4739/6115
🟡 Branches 65.84% 1968/2989
🟡 Functions 71.78% 781/1088
🟡 Lines 77.69% 4471/5755
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🔴
... / eject.ts
0% 0% 0% 0%
🔴
... / translations.ts
0% 0% 0% 0%

Test suite run success

765 tests passing in 108 suites.

Report generated by 🧪jest coverage report action from 46595da

@tatomyr tatomyr force-pushed the feat/add-translations-and-eject branch 5 times, most recently from 068767b to 04f3a21 Compare August 20, 2024 07:40
@tatomyr tatomyr marked this pull request as ready for review August 20, 2024 07:41
@tatomyr tatomyr requested review from a team as code owners August 20, 2024 07:41
@tatomyr tatomyr changed the title feat: add the translations command feat: add the eject and translations commands Aug 20, 2024
@lornajane
Copy link
Contributor

If we are shipping this CLI update without docs (I assume we are, since time is pressing), please open one issue per command for documentation, and link those issues to this pull request. I'll pick them up (we sort of have some old translations docs, but they're not ready to publish as they are)

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.

Added a few comments of things to tidy up before we can publish, trying to make things clear and consistent with existing CLI patterns.

packages/cli/src/index.ts Outdated Show resolved Hide resolved
packages/cli/src/index.ts Outdated Show resolved Hide resolved
packages/cli/src/index.ts Outdated Show resolved Hide resolved
packages/cli/src/index.ts Outdated Show resolved Hide resolved
packages/cli/src/index.ts Outdated Show resolved Hide resolved
packages/cli/src/index.ts Outdated Show resolved Hide resolved
packages/cli/src/index.ts Outdated Show resolved Hide resolved
.changeset/short-panthers-smoke.md Outdated Show resolved Hide resolved
@tatomyr tatomyr force-pushed the feat/add-translations-and-eject branch 3 times, most recently from 40446ea to 67e9b69 Compare August 21, 2024 14:03
@tatomyr tatomyr force-pushed the feat/add-translations-and-eject branch from 4e61e6e to cf81199 Compare August 22, 2024 16:56
docs/commands/preview.md Show resolved Hide resolved
| --plan | string | Product plan to use in preview. <br/> **Possible values:** `pro`, `enterprise`. The default value is `enterprise`. For more details, see [plans](https://redocly.com/pricing/). |
| --product | string | Name of a product to preview the project with. <br/> **Possible values:** `redoc`, `revel`, `reef`, `realm`, `redoc-revel`, `redoc-reef`, `revel-reef`. <br/> `redoc` is the flagship product for generating API documentation from OpenAPI specifications. <br/> `revel` is a specialized product designed for external API applications. <br/> `reef` is a specialized product designed for internal API needs. <br/> `realm` is a balanced product combining `redoc`, `revel`, and `reef`. <br/> `redoc-revel` is a blended product combining `redoc` and `revel`. <br/> `redoc-reef` is a blended product combining `redoc` and `reef`. <br/> `revel-reef` is a blended product combining `revel` and `reef`. <br/> The default value is autodetected from the project's `package.json` or `realm` is used. |
| --project-dir, -d | string | Path to the project directory. The default value is `.` (current directory). |
| --port, -p | number | The port to run the preview server on. The default value is `4000`. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may not be a helpful change for a command that has port, plan and product parameters. Not a blocking request for change, but a general note that we did discuss this at the time that the command was implemented.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the problem with that at all.

Here is an example of a psql (Postgress client) CLI docs:
image

They have multiple args that start with letter p but still use -p as --port.

I also see this very common to in linux CLI tools, e.g. curl -i where -i is an alias for --show-headers while curl has many other arguments starting with letter i: --include, --insecure, --interface.

packages/cli/src/index.ts Outdated Show resolved Hide resolved
@lornajane lornajane dismissed their stale review August 22, 2024 18:57

Avoid blocking progress while I am OOO

"@redocly/cli": minor
---

Added the `eject` and `translations` commands for use with the new Reunite-hosted product family.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to be annoying in the final hour. I think we should rename the translations command to translate. Most of our commands are in verb format:

  • lint
  • bundle
  • eject
  • preview

Why not translate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the corresponding PR for the original source: https://github.com/Redocly/redocly/pull/10425

const npxExecutableName = process.platform === 'win32' ? 'npx.cmd' : 'npx';
spawn(
npxExecutableName,
['-y', '@redocly/realm', 'translations', argv.locale, `--project-dir=${argv['project-dir']}`],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also recommend we rename it in the original source to translate too.

@tatomyr tatomyr changed the title feat: add the eject and translations commands feat: add the eject and translate commands Aug 23, 2024
@RomanHotsiy RomanHotsiy merged commit 78e0881 into main Aug 23, 2024
31 checks passed
@RomanHotsiy RomanHotsiy deleted the feat/add-translations-and-eject branch August 23, 2024 07:25
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.

Translations and eject command wrapper
6 participants