-
-
Notifications
You must be signed in to change notification settings - Fork 238
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 support for remote templates #261
feat: Add support for remote templates #261
Conversation
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 had some issues trying to generate with our templates, check out my comments.
General comments:
- if we do not want to have
fore-install
flag, should we still refer to it in the code asforce
installation? like* @param {Boolean} [options.forceInstall=false] Force the installation of the template and its dependencies.
It is a bit confusing - readme needs some love. There is no information about remote and local templates. There is no info about the fact that we support npm/git/local/tar. In related GitHub issue you have it well explained/listed. I think we need to:
- extend https://github.com/asyncapi/generator/pull/261/files#diff-04c6e90faac2675aa89e2176d2eec7d8R37 and not only describe options but also
<asyncapi>
and<template>
. This is also needed because we do not support remote location of the asyncapi file, only remote template. Btw maybe we should support remote asyncapi file too - examples should be extended with above, like example how to generate with template stored in git repo
- extend https://github.com/asyncapi/generator/pull/261/files#diff-04c6e90faac2675aa89e2176d2eec7d8R37 and not only describe options but also
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 addded just few comments, and also what about the my comment to forceInstall?
when I checked with git and tar, the installation is always triggered, even if I already used the template once:
wookiee$ ./cli.js ./test/docs/streetlights.yml git://github.com/asyncapi/html-template.git -o ./output --force-write
npm http fetch GET 304 https://registry.npmjs.org/markdown-it 712ms (from cache)
npm http fetch GET 304 https://registry.npmjs.org/openapi-sampler 796ms (from cache)
npm http fetch GET 304 https://registry.npmjs.org/entities 167ms (from cache)
npm http fetch GET 304 https://registry.npmjs.org/json-pointer 165ms (from cache)
npm http fetch GET 304 https://registry.npmjs.org/foreach 151ms (from cache)
+ @asyncapi/html-template@0.2.0
updated 1 package in 7.884s
21 packages are looking for funding
run `npm fund` for details
Done! ✨
Check out your shiny new generated files at /Users/wookiee/Documents/sources/generator-add-support-for-remote-templates/output.
wookiee$ ./cli.js ./test/docs/streetlights.yml git://github.com/asyncapi/html-template.git -o ./output --force-write
npm http fetch GET 304 https://registry.npmjs.org/openapi-sampler 622ms (from cache)
npm http fetch GET 304 https://registry.npmjs.org/markdown-it 648ms (from cache)
npm http fetch GET 200 https://registry.npmjs.org/entities 5ms (from cache)
npm http fetch GET 304 https://registry.npmjs.org/json-pointer 157ms (from cache)
npm http fetch GET 304 https://registry.npmjs.org/foreach 169ms (from cache)
+ @asyncapi/html-template@0.2.0
updated 1 package in 7.24s
21 packages are looking for funding
run `npm fund` for details
Done! ✨
Check out your shiny new generated files at /Users/wookiee/Documents/sources/generator-add-support-for-remote-templates/output.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@derberg all comments have been addressed now.
In order to know the name of the package —and know if it has been installed already— npm has to first download it to look into the package.json file. That's why it's always downloaded :) |
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.
Checked functionality with all possible cases:
- tar works like a charm
./cli.js ./test/docs/streetlights.yml https://github.com/asyncapi/html-template/archive/v0.2.0.tar.gz -o ./output --force-write
- npm works like a charm
./cli.js ./test/docs/streetlights.yml @asyncapi/html-template -o ./output -p title='Hello from param' --force-write
- github works like a charm
./cli.js ./test/docs/streetlights.yml https://github.com/asyncapi/nodejs-template -o ./output -p server=production --force-write
- local works like a charm
./cli.js ./test/docs/streetlights.yml ../nodejs-template -o ./output -p server=production --force-write
- npm with a specific version works like a charm
./cli.js ./test/docs/streetlights.yml @asyncapi/html-template@0.1.0 -o ./output -p title='Hello from param' --force-write
- github with a specific tag works like a charm
./cli.js ./test/docs/streetlights.yml git://github.com/asyncapi/html-template.git#v0.1.0 -o ./output --force-write
Works like a charm.
This feature is a killer. Well done! 🚀
🎉 This PR is included in version 0.35.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Changes this PR introduces:
-w, --watch
flag to--watch-template
. Let's leave-w, --watch
for "watching" the AsyncAPI file. Shows a warning when the template directory undernode_modules
is not a symlink (i.e., it's a remote template).--force-install
. Its name is-i, --install
now. It will install the template and its dependencies.-t, --templates <templateDir>
flag. Keep reading to know why.This is how it feels now on the CLI:
Related issue(s)
Fixes #251
To do