cli icon indicating copy to clipboard operation
cli copied to clipboard

feat: integrate `cupid` in cli using the `relation` command

Open arjungarg07 opened this issue 3 years ago • 15 comments

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

arjungarg07 avatar Nov 22 '21 17:11 arjungarg07

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 ...

fmvilas avatar Dec 02 '21 19:12 fmvilas

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.

arjungarg07 avatar Dec 03 '21 16:12 arjungarg07

🤔 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?

fmvilas avatar Dec 07 '21 18:12 fmvilas

What about asyncapi generate relations ?

arjungarg07 avatar Dec 07 '21 19:12 arjungarg07

🤔 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?

fmvilas avatar Dec 08 '21 15:12 fmvilas

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

arjungarg07 avatar Dec 09 '21 20:12 arjungarg07

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

fmvilas avatar Dec 10 '21 15:12 fmvilas

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

sonarqubecloud[bot] avatar Jan 18 '22 10:01 sonarqubecloud[bot]

What about asyncapi relate?

relate

to find or show the connection between two or more things

felixjung avatar Jan 18 '22 15:01 felixjung

What about asyncapi relate?

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

arjungarg07 avatar Jan 24 '22 15:01 arjungarg07

@arjungarg07 not sure if this is intuitive. Look at https://github.com/asyncapi/cli/issues/129#issuecomment-998752834. 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

derberg avatar Jan 27 '22 14:01 derberg

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.

fmvilas avatar Jan 27 '22 15:01 fmvilas

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

sonarqubecloud[bot] avatar Mar 05 '22 10:03 sonarqubecloud[bot]

@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

derberg avatar May 24 '22 16:05 derberg

This pull request has been automatically marked as stale because it has not had recent activity :sleeping:

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 :heart:

github-actions[bot] avatar Sep 22 '22 00:09 github-actions[bot]

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

derberg avatar Nov 17 '22 15:11 derberg

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

arjungarg07 avatar Nov 27 '22 10:11 arjungarg07

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 avatar Nov 28 '22 19:11 derberg

@arjungarg07 yo, should we keep it open?

derberg avatar Mar 27 '23 07:03 derberg

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

arjungarg07 avatar Mar 27 '23 11:03 arjungarg07

up to you 😃

derberg avatar Mar 28 '23 16:03 derberg

to be resolved(asked above in the thread)

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

derberg avatar Apr 03 '23 16:04 derberg

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

derberg avatar Jul 19 '23 11:07 derberg

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

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

chinma-yyy avatar Jul 29 '23 17:07 chinma-yyy

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?

derberg avatar Jul 31 '23 10:07 derberg

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

derberg avatar Sep 12 '23 08:09 derberg