-
-
Notifications
You must be signed in to change notification settings - Fork 232
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: add documents and postinstallation scripts for ag deprecation #1251
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 9e7f010 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Looking Good @lmgyuan |
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.
@lmgyuan looking good, just left few suggestions. Also please review entire PR and be consistent and replace mention of Cli.js
with proper ag
mention (even in PR title)
README.md
Outdated
@@ -11,6 +11,9 @@ This is a Monorepo managed using [Turborepo](https://turbo.build/) and contains | |||
|
|||
> warning: This package doesn't support AsyncAPI 1.x anymore. We recommend to upgrade to the latest AsyncAPI version using the [AsyncAPI converter](https://github.com/asyncapi/converter-js) (You can refer to [installation guide](/apps/generator//docs//installation-guide.md)). If you need to convert documents on the fly, you may use the [Node.js](https://github.com/asyncapi/converter-js) or [Go](https://github.com/asyncapi/converter-go) converters. | |||
|
|||
> **Deprecation Notice**: The use of `cli.js` for documentation generation is deprecated and will be removed in future releases. We strongly encourage migrating to the new AsyncAPI CLI. The migration process is straightforward, and you can find the necessary steps in our [migration guide](/apps/generator/docs/deprecate-cli.sj-ag.md). Please ensure that your projects are updated accordingly to avoid any disruptions. |
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.
remember that users do not understand waht cli.js
is, only maintainers. This notice is for users and they only know that cli is ag
or asyncapi-generator
also please use one of GH supported markdown alerts -> https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax#alerts. Probably warning
is what we need.
and last but no least:
- link
/apps/generator/docs/deprecate-cli.sj-ag.md
transform to a link pointing tohttps://asyncapi.com/docs/tools/generator/cli-deprecation
- yes, link is 404 but will not be after we merge this PR. Of course you need to renamedeprecate-cli.sj-ag.md
file tocli-deprecation.md
- please provide precise information to this note that removal of functionality will be done in September 2025
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.
@@ -0,0 +1,85 @@ | |||
--- | |||
title: "Deprecate Cli.js" |
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.
as in my other comment, it is not a source file that we deprecate but the AsyncAPI Generator CLI that has different technical name. See how others still promote it in template readme files -> https://github.com/asyncapi/java-spring-cloud-stream-template?tab=readme-ov-file#how-to-use-this-template as ag
- **Consistency:** Ensures consistent command usage across different environments. | ||
- **Support and Maintenance:** Future updates and support will focus on the AsyncAPI CLI. | ||
|
||
## Deprecated `ag/asyncapi-generator` Options and Their AsyncAPI CLI Equivalents |
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.
great work of preparing these equivalents!!!
## Migration Steps | ||
|
||
### 1. Install AsyncAPI CLI | ||
First, ensure you have the AsyncAPI CLI installed: |
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.
please add note that there are different methods that do not require setup of node environment -> https://www.asyncapi.com/docs/tools/cli/installation
the best probably would be to use https://github.com/asyncapi/website/blob/master/assets/docs/fragments/cli-installation.md fragment - this is how to use it -> https://github.com/asyncapi/website/blob/master/markdown/docs/tutorials/generate-code.md?plain=1#L32
|
||
**After Migration (Using AsyncAPI CLI)**: | ||
``` | ||
asyncapi generate fromTemplate ./asyncapi.yaml ./template --output ./output --param param1=value1 --debug --install --disable-hook hookType=hookName |
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.
if above ag
example as -o
or -p
wouldn't it be better to also show the same here? instead of --output
and --param
.
### 3. Verify and Test | ||
Run the updated commands to ensure they work as expected. Verify the output and ensure that all files are generated correctly. | ||
|
||
### 4. Enable Watch Mode (Optional) |
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'm not sure we really need this one, even if optional
Example Migration: | ||
|
||
Before Migration (Using 'Cli.js'): | ||
$ node Cli.js ./asyncapi.yaml ./template -o ./output -p param1=value1 --debug --install --disable-hook hookType=hookName |
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.
not node Cli.js
Please migrate to the new AsyncAPI CLI using the following guide: | ||
|
||
1. Install AsyncAPI CLI: | ||
$ npm install -g @asyncapi/cli | ||
|
||
2. Update your commands: | ||
Replace the deprecated 'Cli.js' commands with their AsyncAPI CLI equivalents. |
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.
better just link to the deprecation guide
@@ -25,7 +25,8 @@ | |||
"lint:tpl:validator": "eslint --fix --config ../../.eslintrc ../../.github/templates-list-validator", | |||
"generate:readme:toc": "markdown-toc -i README.md", | |||
"generate:assets": "npm run docs && npm run generate:readme:toc", | |||
"bump:version": "npm --no-git-tag-version --allow-same-version version $VERSION" | |||
"bump:version": "npm --no-git-tag-version --allow-same-version version $VERSION", | |||
"postinstall": "node ./scripts/postinstall.js" |
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 know we talked about postinstallation as best option, but to be honest, I'm still in doubts. This notice will show up every-time someone installs generator, even if it is a dependency to AsyncAPI CLI - so during AsyncAPI CLI installation - at least I think so - something you would have to verify
this means we will spam all generator users, even the ones not using ag
🤔 maybe in the end it is better to have a warning invoked only as part of cli.js code? even if it runs every time someone invokes it? 🤔
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.
hmmmm my concern with having a warning only for invoking cli.js is that it would not be obvious? Since a lot of users probably already have cli.js as part of their code's workflow and don't really look at it anymore? But I can see this is also a problem for having a message during installation.
If spamming users is a problem, I can shorten the message a bit so it would be as annoying. What do you think?
@lmgyuan hey, any updates |
- delete unnecessary warning - add more context to the changeset documentation. - modify deprecate-cli.js-ag.md - -o and -p instead of --output and --param - delete enable watch mode part in the deprecation readMe - delete replace cli.js in postinstall.js with ag - add a link to the deprecation guide in the postinstall.js - add an installation url in the postinstall.js
@derberg I just updated the PR. We can discuss this on Wednesday. |
added missing ;
"@asyncapi/generator": minor | ||
--- | ||
|
||
Start deprecating `ag` in the generator repository. The purpose of deprecating the `ag` option for documentation generation is to move people's |
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.
it's not much about moving attention. The reason of deprecation is to not remove without proper notice and without giving people time to migrate.
|
||
Start deprecating `ag` in the generator repository. The purpose of deprecating the `ag` option for documentation generation is to move people's | ||
attention to use the `AsyncAPI CLI` in order to maintain a single entry point for all the AsyncAPI tools. | ||
Otherwise, the maintainers need to manually update the `ag` option in the generator repository every time a new version of the `AsyncAPI CLI` is released. |
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.
what do you mean?
@@ -9,7 +9,8 @@ This is a Monorepo managed using [Turborepo](https://turbo.build/) and contains | |||
|
|||
![npm](https://img.shields.io/npm/v/@asyncapi/generator?style=for-the-badge) ![npm](https://img.shields.io/npm/dt/@asyncapi/generator?style=for-the-badge) | |||
|
|||
> warning: This package doesn't support AsyncAPI 1.x anymore. We recommend to upgrade to the latest AsyncAPI version using the [AsyncAPI converter](https://github.com/asyncapi/converter-js) (You can refer to [installation guide](/apps/generator//docs//installation-guide.md)). If you need to convert documents on the fly, you may use the [Node.js](https://github.com/asyncapi/converter-js) or [Go](https://github.com/asyncapi/converter-go) converters. | |||
[!IMPORTANT] | |||
Deprecation Notice: The use of ag for documentation generation is deprecated and will be removed in future releases. We strongly encourage migrating to the new AsyncAPI CLI. |
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.
Deprecation Notice: The use of ag for documentation generation is deprecated and will be removed in future releases. We strongly encourage migrating to the new AsyncAPI CLI. | |
Deprecation Notice: The use of the Generator CLI (known as `ag`) for generation is deprecated and will be removed in future releases. We strongly encourage migrating to the new AsyncAPI CLI. |
also make migrating to the new AsyncAPI CLI
a link that will point to migration document. You can predict the URL of that document once it is published to the website. Just examine other markdown docs, their versions in website, and notice how URL is mapped to file name
@@ -0,0 +1,79 @@ | |||
--- | |||
title: "Deprecate ag" |
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 migration document, so better call it like @Gmin2 his doc for migrating Nunjucks, so title related to content
also update name of file, and better not use cli.js
in title, I'm not sure how extra .
in the filename will be reflected in url of the document link in the website
weight: 170 | ||
--- | ||
|
||
# Migration Guide from “ag/asyncapi-generator” to AsyncAPI CLI |
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.
no need for h1 here, this should be handled by title
, so please remove it and make nicer title
Quality Gate passedIssues Measures |
Description
cli.js
andag
toAsyncAPI CLI
postinstall.js
to generate post-installation script after installing the generator package.Related issue(s)
Resolves #1229