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

feat: integrate asyncapi optimize library using the optimize command #255

Closed
wants to merge 2 commits into from

Conversation

alceil
Copy link

@alceil alceil commented Mar 11, 2022

Description

This PR integrates the asyncapi-optimize library using the optimize command.

At the moment, the command looks something like this.

asyncapi optimize <context/filepath/url>

And it has flag:
--format which determines the show the report or not.

Related issue(s)
Fixes #218

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide 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.

@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
No Duplication information No Duplication information

@KhudaDad414
Copy link
Member

Thanks, @alceil looks great.
just some inputs from my side:

when I use the optimize command, I expect it to show me the changes that are going to be applied and I should be able to chose between those changes.

this is the flow that I expect to go through:

# asyncapi optimize document.yml
Here is a list of optimizations that can be done on document.yml:

Remove the following components:
 - components.messages.foo
 - components.schemas.bar
...
 
Reuse the following components:
 - components.schemas.foo in channels.someMessge.publish.message.payload
 - components.schemas.bar in channels.someMessge.publish.message.payload
...

Move the following components to the components section:
 - channels.foo.publish.message to channels.someMessge.publish.message.payload
 - channels.foo.subscribe.headers[0] to components.schemas.schema_1
...

Select your desired optimization to be applied:
   Delete Components
   Remove Components
   Reuse Components
> All of them

select your desired output:
   Update the current file
> Create document_optimized.yml (Recommended)
   Log to console

document_optimized.yml has been created!

we definitely need some input about the UX from others here.
cc: @derberg @magicmatatjahu @Souvikns

@alceil
Copy link
Author

alceil commented Mar 12, 2022

Can I also get some inputs on the Ux as well. @KhudaDad414 . Im pretty new to this so I need some pointers as well on how to approach it

@derberg
Copy link
Member

derberg commented Mar 14, 2022

from the UX perspective we need to support 2 ways of doing things:

  • interactivity, step by step guiding user. As shown by @KhudaDad414, that you show what can be optimized and user can select what changes should be applied
  • flags to skip interactivity in case CLI is used on a CI/CD. So user should be able to specify with flags desired out come + should be powered by defaults. So for example, default should be that if --no-tty is passed document is optimized, but the result is logged to terminal, and you can as alternative specify --override or --output flags to save optimized document into file.

does it make sense?

@alceil
Copy link
Author

alceil commented Mar 17, 2022

Okay got it @derberg

@derberg
Copy link
Member

derberg commented Apr 4, 2022

@alceil do you plan to continue working on it? Open Force ended but since you started during the event, it is ok if you complete work later

@alceil
Copy link
Author

alceil commented Apr 4, 2022

Sure i wish to complete it .

@imabp
Copy link
Member

imabp commented Apr 20, 2022

Can I also get some inputs on the Ux as well. @KhudaDad414 . Im pretty new to this so I need some pointers as well on how to approach it

@alceil for UX, we are following CLIG, as mentioned in readme.

As @KhudaDad414 mentioned, @alceil . I guess you are working on it, feel free if you feeling stuck anywhere. An idea that you can think for the implementation is using a stepper design.

  1. First step is to just look on the optimizations possible and show users a list of possible optimizations in tabular format(more readable way).

  2. Second step is to let the user choose from the given four options, reuse, remove, move, and All of them

  3. Third step is to ask the updated file preferences with these options
    Update the current file, Create document_optimized.yml (Recommended)
    and Log to console

@derberg
Copy link
Member

derberg commented Jul 5, 2022

@alceil hey, this PR is opened for some time already, maybe we should close and you create new when ready?

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

This pull request 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 pull request, add a comment with detailed explanation.

There can be many reasons why some specific pull request 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 pull request 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 Nov 3, 2022
@derberg
Copy link
Member

derberg commented Nov 17, 2022

@KhudaDad414 hey, looks like oryginal contributor do not plan to continue with this one. Did you plan to integrate optimizer here in the CLI?

@KhudaDad414
Copy link
Member

I am putting it on my to-do list. thanks for reminding @derberg.

@github-actions github-actions bot removed the stale label Nov 18, 2022
@KhudaDad414
Copy link
Member

I guess we should close this PR in favour of #397

@derberg
Copy link
Member

derberg commented Nov 22, 2022

closing since @alceil is no longer active here and @KhudaDad414 opened new PR

@derberg derberg closed this Nov 22, 2022
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.

Add asyncapi optimize command
4 participants