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: add documents and postinstallation scripts for ag deprecation #1251

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

lmgyuan
Copy link
Collaborator

@lmgyuan lmgyuan commented Aug 10, 2024

Description

  • Add a migration guide markdown to transition from cli.js and ag to AsyncAPI CLI
  • Add a postinstall.js to generate post-installation script after installing the generator package.
  • update the homepage ReadMe to notify this change.

Related issue(s)

Resolves #1229

Copy link

changeset-bot bot commented Aug 10, 2024

🦋 Changeset detected

Latest commit: 6ab60db

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

This PR includes changesets to release 1 package
Name Type
@asyncapi/generator 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

@Gmin2
Copy link
Collaborator

Gmin2 commented Aug 11, 2024

Looking Good @lmgyuan

README.md Outdated Show resolved Hide resolved
Copy link
Member

@derberg derberg left a 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)

.changeset/start_clijs_deprecation.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
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.
Copy link
Member

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 to https://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 to cli-deprecation.md
  • please provide precise information to this note that removal of functionality will be done in September 2025

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the GH supported markdown alerts, do you mean this?
Screenshot 2024-09-16 at 2 26 27 PM

@@ -0,0 +1,85 @@
---
title: "Deprecate Cli.js"
Copy link
Member

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
Copy link
Member

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:
Copy link
Member

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
Copy link
Member

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)
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

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

not node Cli.js

Comment on lines 10 to 16
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.
Copy link
Member

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"
Copy link
Member

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? 🤔

Copy link
Collaborator Author

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?

@derberg
Copy link
Member

derberg commented Sep 16, 2024

@lmgyuan hey, any updates

@lmgyuan lmgyuan changed the title docs: add documents and postinstallation scripts for Cli.js deprecation docs: add documents and postinstallation scripts for ag deprecation Sep 16, 2024
- 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
@lmgyuan
Copy link
Collaborator Author

lmgyuan commented Sep 16, 2024

@derberg I just updated the PR. We can discuss this on Wednesday.

added missing ;
Copy link

sonarcloud bot commented Sep 16, 2024

"@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
Copy link
Member

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.
Copy link
Member

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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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"
Copy link
Member

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

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.

Migration Guide to md & post-installation script to deprecate cli.js
4 participants