Skip to content

feat: integrate cupid in cli using the relation command#143

Closed
arjungarg07 wants to merge 9 commits into
asyncapi:masterfrom
arjungarg07:feat/cupidIntegration
Closed

feat: integrate cupid in cli using the relation command#143
arjungarg07 wants to merge 9 commits into
asyncapi:masterfrom
arjungarg07:feat/cupidIntegration

Conversation

@arjungarg07

Copy link
Copy Markdown

Description

This PR integrates @asyncapi/cupid in cli using the relation command.

Command usage syntax:
asyncapi relation <context-1/filepath1> <context-2/filepath2> <context-3/filepath3> ...

It has one flag:
--type which determines the output syntax of the relations and if not provided, would output default output syntax.

Related issue(s)
#47

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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.

@fmvilas

fmvilas commented Dec 2, 2021

Copy link
Copy Markdown
Contributor

Great stuff, @arjungarg07! My only concern is that the command is not a verb. We decided to follow the form asyncapi VERB [NOUN] and relation is not a verb. What about visualize?

asyncapi visualize context1/asyncapi1 context2/asyncapi2 ...

@arjungarg07

arjungarg07 commented Dec 3, 2021

Copy link
Copy Markdown
Author

Yeah @fmvilas , I thought of using the verb visualize but I was skeptical about it since we are not actually visualizing the architecture right away but just getting the relations in desired output format and later pasting it in live editor to visualize.

@fmvilas

fmvilas commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

🤔 I see what you mean. Maybe it should be something like asyncapi generate diagram? It kinda collapses with asyncapi generate where we probably want to use Generator. But maybe we can use asyncapi generate code and asyncapi generate docs for those...

For sure it can't be relation as it's not a verb/command, just a noun. Any other ideas?

@arjungarg07

Copy link
Copy Markdown
Author

What about asyncapi generate relations ?

@fmvilas

fmvilas commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

🤔 It might sound as you're generating the relations but the relations are already there, you're just "finding" them and generating a diagram, isn't it?

@arjungarg07

Copy link
Copy Markdown
Author

I didn't think in that way, in that case asyncapi generate diagram would be good.

@fmvilas

fmvilas commented Dec 10, 2021

Copy link
Copy Markdown
Contributor

Yeah, now what worries me is that clash with asyncapi generate command. There's a related conversation going on in #129.

@sonarqubecloud

Copy link
Copy Markdown

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@felixjung

felixjung commented Jan 18, 2022

Copy link
Copy Markdown

What about asyncapi relate?

relate

to find or show the connection between two or more things

@arjungarg07

Copy link
Copy Markdown
Author

What about asyncapi relate?

Yeah, this could be done.
@derberg what do you think about it?

@derberg

derberg commented Jan 27, 2022

Copy link
Copy Markdown
Member

@arjungarg07 not sure if this is intuitive. Look at #129 (comment). I think it should go under generate because in the end you are generating something. It is about the outcome and not what the library is doing. You tell CLI to generate a diagram, and not find relations. At least this is how I look at it 🤔

@fmvilas have a look as you were involved here

@fmvilas

fmvilas commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

Yeah, I still think it's better if we stick to generate diagram or similar. Always look at it from the point of view of the user who doesn't know the underlying technology and instead they expect the command to represent what they want to achieve. The tool used for this is irrelevant for them.

@Souvikns Souvikns mentioned this pull request Mar 3, 2022
16 tasks
@sonarqubecloud

sonarqubecloud Bot commented Mar 5, 2022

Copy link
Copy Markdown

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 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@derberg

derberg commented May 24, 2022

Copy link
Copy Markdown
Member

@arjungarg07 hey is this one ready for review?

  • can you remove 2 code smells?
  • can you remove manual bump of the cli version from package.json and package-lock.json, as we do it by the bot, and it will constantly cause issues for you on a PR, conflicts you do not want to have

@github-actions

Copy link
Copy Markdown
Contributor

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 Sep 22, 2022
@derberg

derberg commented Nov 17, 2022

Copy link
Copy Markdown
Member

@arjungarg07 hey, not sure if you noticed but asyncapi generate is now possible, so you can continue with this PR if you want

@github-actions github-actions Bot removed the stale label Nov 18, 2022
@arjungarg07

Copy link
Copy Markdown
Author

@derberg what would be the final command look like? asyncapi generate relation ?

@derberg

derberg commented Nov 28, 2022

Copy link
Copy Markdown
Member

I think mostly suggested was diagram not relation

this could maybe also be integrated with EDAVisualizer @jonaslagoni ?

so one command, but underneath different tools would be used

@derberg

derberg commented Mar 27, 2023

Copy link
Copy Markdown
Member

@arjungarg07 yo, should we keep it open?

@arjungarg07

Copy link
Copy Markdown
Author

@derberg I had some doubts, to be resolved(asked above in the thread). Maybe, would open a new PR.

@derberg

derberg commented Mar 28, 2023

Copy link
Copy Markdown
Member

up to you 😃

@derberg

derberg commented Apr 3, 2023

Copy link
Copy Markdown
Member

to be resolved(asked above in the thread)

oh wait, sorry, which doubts are not yet solved?

@derberg derberg mentioned this pull request Apr 3, 2023
@derberg

derberg commented Jul 19, 2023

Copy link
Copy Markdown
Member

@arjungarg07 I'm guessing we should close it, right?

@chinma-yyy

Copy link
Copy Markdown
Contributor

@arjungarg07 I'm guessing we should close it, right?

So does it mean that this issue is open to other contributors @derberg ??

@derberg

derberg commented Jul 31, 2023

Copy link
Copy Markdown
Member

yes, but since Arjun is the maintainer of cupid, let's first confirm if this issue makes sense.

@arjungarg07 what are your plans with cupid, do you plan to continue maintain it?

@Shurtu-gal Shurtu-gal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also, the example folder can be moved into https://github.com/asyncapi/cli/tree/master/test/fixtures

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@arjungarg07 there has been a recent change in the folder structure for tests in #712. This can be moved into https://github.com/asyncapi/cli/tree/master/test/integration.

@derberg

derberg commented Sep 12, 2023

Copy link
Copy Markdown
Member

closing for now, I'm in touch with @arjungarg07 to start a process of looking for new maintainers for cupid

@derberg derberg closed this Sep 12, 2023
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.

6 participants