Skip to content

Conversation

@Souvikns
Copy link
Member

Description

Integrating bundler into CLI by introducing the command asyncapi bundle.

Usage:

asyncapi bundle ./file1.yml ./file2.yml --output asyncapi.yaml --referenceIntoComponents

Related issue(s)

Resolves #219

@Souvikns Souvikns marked this pull request as ready for review November 17, 2022 08:54
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.

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

@derberg
Copy link
Member

derberg commented Dec 1, 2022

@Souvikns remember to click arrows when ready for another review
Screenshot 2022-12-01 at 14 28 33

@Souvikns Souvikns requested a review from derberg December 1, 2022 13:31
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.

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 🤔

@Souvikns
Copy link
Member Author

Not sure why but now it is unable to install dependencies https://github.com/asyncapi/cli/actions/runs/4163522302/jobs/7204027231#step:7:69

@derberg
Copy link
Member

derberg commented Feb 14, 2023

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

@Souvikns
Copy link
Member Author

@derberg could this be a ubuntu related issues, on mac at least npm i works

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.

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

@derberg
Copy link
Member

derberg commented Mar 27, 2023

@Souvikns yo, with all the recent changes here and there, try fixing conflicts and maybe 🤞🏼 this time will be good

@derberg
Copy link
Member

derberg commented Mar 30, 2023

@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

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Souvikns
Copy link
Member Author

@derberg finally it works 😄

@Souvikns Souvikns requested a review from derberg March 30, 2023 15:49
@derberg
Copy link
Member

derberg commented Mar 30, 2023

/rtm

@asyncapi-bot asyncapi-bot merged commit 05cb674 into asyncapi:master Mar 30, 2023
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.36.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add asyncapi bundle command

3 participants