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

Integrate asyncapi/diff in the CLI #58

Closed
Tracked by #251
aayushmau5 opened this issue Aug 30, 2021 · 12 comments
Closed
Tracked by #251

Integrate asyncapi/diff in the CLI #58

aayushmau5 opened this issue Aug 30, 2021 · 12 comments
Labels
enhancement New feature or request

Comments

@aayushmau5
Copy link
Member

Since the asyncapi/diff library is, now, usable, we can integrate it into the AsyncAPI CLI.

My idea for the command usage:

asyncapi diff --files <file1.yml> <file2.yml>
asyncapi diff --contexts file1 file2

So, we need to think about some things:

  • How to output the data
  • Where to output the data
  • The flags which will specify the type of output

How to output the data:

For this, I'm thinking of having a flag --<format>. For started, we can provide output in JSON format. Then, in the future, we can go to HTML based output, markdown, etc.

asyncapi diff --files <file1.yml> <file2.yml> --json
asyncapi diff --files <file1.yml> <file2.yml> --html

Where to output the data:

Now the question arises, where do we output the data. In a file? In the terminal?
Looking at OpenAPI diff tool, we can also provide an optional <file> option along with --<format> flag. --<format> <file>. The default output will be in the terminal.

asyncapi diff --files <file1.yml> <file2.yml> --json <file-output>
asyncapi diff --files <file1.yml> <file2.yml> --html <file-output>

The flags which will specify the type of output:

Since the diff library provides a bunch of helpers function which provide different output(ex. only breaking changes, only non-breaking changes), we can provide a flag to do this through the CLI. The default will provide the full output.
ex.

asyncapi diff --files <file1.yml> <file2.yml> --json <file> --breaking # for breaking changes
asyncapi diff --files <file1.yml> <file2.yml> --json <file> --non-breaking # for non-breaking changes

Any thoughts on this idea?

@aayushmau5 aayushmau5 added the enhancement New feature or request label Aug 30, 2021
@github-actions
Copy link
Contributor

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.

Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@derberg
Copy link
Member

derberg commented Aug 30, 2021

Hey, there is a topics covered here that are important no matter if we work on diff or not: the output

atm, library outputs on terminal only. There is no --output option so far. This is independent from diff as we need output to file functionality in many different commands, for relations finding or for optimizations. This needs to be implemented separately

regarding optional flags, we so far tend to always have name, so instead of --json do --format=json

as if it comes to asyncapi diff --files <file1.yml> <file2.yml>, we have some discussions in other issue that are kind of going the direction of getting rid of --file or --files flag, as flags are options, they are optional, while in case of file, it is must have, not optional, it is kinda a center of all the things

@aayushmau5
Copy link
Member Author

atm, library outputs on terminal only. There is no --output option so far. This is independent from diff as we need output to file functionality in many different commands, for relations finding or for optimizations. This needs to be implemented separately

Got it.

regarding optional flags, we so far tend to always have name, so instead of --json do --format=json

👍

as if it comes to asyncapi diff --files <file1.yml> <file2.yml>, we have some discussions in other issue that are kind of going the direction of getting rid of --file or --files flag, as flags are options, they are optional, while in case of file, it is must have, not optional, it is kinda a center of all the things

So I guess, we will have

asyncapi diff <file1> <file2l>

for file.

@derberg
Copy link
Member

derberg commented Aug 30, 2021

So I guess, we will have
asyncapi diff <file1> <file2l>
for file.

yup, but with clear indication which one is checked against the other, I mean if file1 is checked against file 2? or the opposite

the part about file flag I mentioned (that it will be removed) is something that @Souvikns will work on #37 (comment)

@aayushmau5
Copy link
Member Author

aayushmau5 commented Nov 8, 2021

Now that the migration to Oclif is done, I will start working on this. Hope to make a PR by the end of this week.

@Souvikns
Copy link
Member

Souvikns commented Nov 8, 2021

Yeah sure, let me know if you need anything.

@aayushmau5
Copy link
Member Author

@Souvikns Does the CLI have the ability to output data into files? Since, Lukasz said that this functionality should be a separate implementation independent of the command.

@Souvikns
Copy link
Member

hmm, it doesn't.

Lukasz said that this functionality should be a separate implementation independent of the command.

Yeah, you should create a wrapper around diff and connect it with CLI. For a little context, the CLI is this, so you just need to create a function wrapping your diff library and create a new command for it.

Can you give me some context on what kind of output diff is generating? As of now, all output is printed in the console itself.

@aayushmau5
Copy link
Member Author

Can you give me some context on what kind of output diff is generating? As of now, all output is printed in the console itself.

Right now I'm just generating the default json output.

@magicmatatjahu
Copy link
Member

Solution that we should consider for that issue asyncapi/community#249 It's only an idea so please treat it as suggestion, not final approach. Feel free to comment :)

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jun 2, 2022
@aayushmau5
Copy link
Member Author

Should we close this issue now that we have integrated diff in CLI?

@github-actions github-actions bot removed the stale label Jun 4, 2022
@derberg derberg closed this as completed Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants