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: added the migration guide for depreciation of nunjucks #1253

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

Conversation

Gmin2
Copy link
Collaborator

@Gmin2 Gmin2 commented Aug 11, 2024

Description
Add a migration guide markdown to migrate from nunjucks to react render engine

Related to #1255

Copy link

changeset-bot bot commented Aug 11, 2024

🦋 Changeset detected

Latest commit: 826f203

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

Copy link

sonarcloud bot commented Aug 11, 2024

@Gmin2
Copy link
Collaborator Author

Gmin2 commented Aug 11, 2024

Does i also remove the code that is present for nunjucks render engine too in this PR

cc @derberg @lmgyuan

Copy link
Member

derberg commented Aug 12, 2024

no, we deprecate it now, inform people that removal will take place in a year. So critical is to inform when and what functionality is removed, and how to migrate to alternative - react

Copy link
Collaborator

@Florence-Njeri Florence-Njeri left a comment

Choose a reason for hiding this comment

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

Hey @Gmin2 I have left a few suggestions that you can apply to the doc. The only things I noted are:

  1. We use sentence case for all our docs heading
  2. Ensure consistency in docs especially what you capitalize e.g if Nunjucks is capital in one place, let make it capitalized everywhere

apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved

### 5. File template

//TODO: we can add a link to Florence docs once it is merged
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @Gmin2 You can already link the doc since I updated the current File template doc

apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.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.

  • have a look on PR from @lmgyuan about cli deprecation. We need similar detailed information here, about deprecation of nunjucks september 2025
  • great we have migration guide, but we also need warning is other generator documents, where nunjucks is explained, and provide clear short note about deprecation, that will link to your guide
  • in migration guide please also point people to https://github.com/asyncapi/template-for-generator-templates and explain that in master branch they see all react-features in use and if they switch to nunjucks branch, they see old features. Basically you did not cover all the features, so safe is to link there as well
  • in guide please mention that nunjucks filters that people placed in filters dir can still be used but as normal functions that you import in your components, and therefore we also recommend to rename them to helpers.
  • in guide mention that hooks stay as they are - no change there as it is not render engine related functionality
  • also in intro of migration guide, and probably also in description of release, please link to https://www.asyncapi.com/blog/react-as-generator-engine article and introduce as an article why we introduced react in 2021 as alternative, and that the same principles apply to remove nunjucks entirely
  • proper code must also be marked as deprecated: https://jsdoc.app/tags-deprecated

@Florence-Njeri in case of CLI deprecation, we no longer mention old ag in docs, only in migration guide. Maybe we should do the same with nunjucks? maybe to make it much easier to maintain documentation, all documentation should be updated and mention react only - except of the guide from @Gmin2 ? For example we probably should:

thoughts?

apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
apps/generator/docs/nunjucka-depreciate.md Outdated Show resolved Hide resolved
}
```

### 4. Macros
Copy link
Member

Choose a reason for hiding this comment

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

@derberg
Copy link
Member

derberg commented Sep 16, 2024

any updates @Gmin2 ?

@Gmin2
Copy link
Collaborator Author

Gmin2 commented Sep 18, 2024

  • have a look on PR from @lmgyuan about cli deprecation. We need similar detailed information here, about deprecation of nunjucks september 2025

@derberg dont you think this needed to be in the issue itself, not in the PR, as this is a migration guide

thoughts @lmgyuan @Florence-Njeri ?

@derberg
Copy link
Member

derberg commented Sep 18, 2024

@Gmin2 that was 3 weeks ago so I don't 100% remember what I was referring to, but afaik, it was about the README.md

Copy link

sonarcloud bot commented Sep 29, 2024

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.

3 participants