-
-
Notifications
You must be signed in to change notification settings - Fork 297
feat: add bundle command #391
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
Conversation
derberg
left a comment
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 left some comments. Can we agree not to use sync-like functions in base code. In tests it is fine but in main CLI code I would prefer we always use async/await style
|
@Souvikns remember to click arrows when ready for another review |
derberg
left a comment
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.
Yo left some comments.
There is also something I'm not 100% sure how we should approach, what is the best.
The things is about read/write of files in the CLI. This is kinda getting out of hands, in some commands we use fs through promises, and here you use sync functions. My opinion is we should use async/await approach across all commands, no sync functions. We basically should do it like @KhudaDad414 here https://github.com/asyncapi/cli/pull/397/files
The only way to assure ☝🏼 is to have own wrapper utils I think, and during review make sure they are used. But I'm not sure what should be the scope for this PR. Maybe we should leave it as it is and have separate followup refactor issue 🤔
|
Not sure why but now it is unable to install dependencies https://github.com/asyncapi/cli/actions/runs/4163522302/jobs/7204027231#step:7:69 |
|
I think it is random npm issue, saw it few times already in other projects -> https://github.com/asyncapi/cli/actions/runs/4163522302/jobs/7204027231#step:7:41 I rerun, let's see |
|
@derberg could this be a ubuntu related issues, on mac at least |
derberg
left a comment
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 makes no sense 😭
maybe try to bump rimraf to latest -> 4.1.2
we use it only in the npm scripts so we should not be probably afraid of the major changes that version 4 brought
|
@Souvikns yo, with all the recent changes here and there, try fixing conflicts and maybe 🤞🏼 this time will be good |
|
@Souvikns you have them there again. Remember to ping me once you solve them. Rerequest review using GitHub buttons. I do not get notifications that you pushed code |
|
Kudos, SonarCloud Quality Gate passed!
|
|
@derberg finally it works 😄 |
|
/rtm |
|
🎉 This PR is included in version 0.36.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |









Description
Integrating bundler into CLI by introducing the command
asyncapi bundle.Usage:
Related issue(s)
Resolves #219