feat: add new `convert` command to CLI #188

peter-rr · 2022-01-20T00:43:23Z

Description

First proposal to implement asyncapi convert command.

Related issue(s)

Add convert command #41

fmvilas

This is looking great, Peter! 🙌

Left a few comments where you can improve the code. If I'm not mistaken, this should already work if you try in your machine 🎉

src/commands/convert.ts

Adding `flags` when parsing `Convert` class to avoid using `Convert.flags` in the rest of the file. Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Using an object as 3rd argument for `convert()` Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Removing `Convert.x.x` notation Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Removing one more `Convert.x.x` Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Printing the content of `convertedFile` properly Co-authored-by: Fran Méndez <fmvilas@gmail.com>

peter-rr · 2022-01-25T21:52:02Z

@fmvilas All requested changes applied and seem to be working fine 👍 I think we're almost ready to start testing the convert command with real asyncapi files 🚀

peter-rr · 2022-02-03T14:10:02Z

@Souvikns I wonder if you have any clue about an error I'm getting when passing the -h flag. The info displayed by the command when the -h flag is passed is correct, but an unexpected Error: EEXIT: 0 is shown at the bottom:

However, when the --help flag is passed, that error is not shown and everything is displayed correctly.

In order to solve this issue I've been trying to import the flags from @oclif/core as you mentioned in this fix, but then I'm getting this error plugin when executing the command:

Do you know if there is something missing or that I'm not taking into consideration? Thank you!! :)

peter-rr · 2022-02-03T20:48:35Z

@magicmatatjahu @fmvilas I've been testing the --id flag and I didn't get any successful result for none of the different spec file versions:

For 2.x versions: The conversion is successful but the --id flag applied doesn't have any effect on the converted file.
For 1.x versions: The conversion failed since the following error is thrown: Error: Version 1.2.0 is not supported. I guess this is related to the converter 🤔, not to the flag itself.

So, my questions about this flag:

Am I missing something at this flag's implementation? Or am I not considering the right use cases?
Does it make sense to keep using this flag? Or should we consider to remove it from convert command.

What do you think, guys?

Souvikns · 2022-02-04T02:26:51Z

@peter-rr can you confirm where you are seeing this error i.e. in your development environment or production.
I checked your branch and it is working as expected.

Now there is an issue in production where we are getting a warning, the CLI works fine but still throws a warning. You can see #192 for full details. We thought #201 would fix the issue but that is not the case currently I have been working on #203 to migrate oclif to the latest version.

peter-rr · 2022-02-04T21:54:57Z

@Souvikns Yeap, I see both errors in my development environment updated with the latest changes from production. Maybe I'm missing some package or plugin needed, not sure 🤔

I checked your branch and it is working as expected.

Do you mean you didn't get any of those errors when running the command on my branch?

We thought #201 would fix the issue but that is not the case currently I have been working on #203 to migrate oclif to the latest version.

Then I think I should wait until #203 is merged so I can test my branch with the latest oclif version and check if those errors still persist. I'll let you know :)

Souvikns · 2022-02-05T04:29:42Z

Do you mean you didn't get any of those errors when running the command on my branch?

Yeah, I opened your PR in my local setup. Do an npm install and see if it fixes the issue. Do let me know if it doesn't

magicmatatjahu

Awesome, but I have some insight :)

src/commands/convert.ts

peter-rr · 2022-03-04T19:32:41Z

Now I see you have already created a .d.ts file but you have to change the tsconfig.json file. Just add your global.d.ts file inside include array

Still getting the same errors after applying that change. I guess there is something I'm doing wrong or not taking into account 🤔

Souvikns · 2022-03-07T09:38:26Z

src/global.d.ts

@@ -0,0 +1,2 @@
+declare module '@asyncapi/converter';


Suggested change

declare module '@asyncapi/converter';

declare module '@asyncapi/converter' {

export async function convert(spec: string, targetVersion: string, options: any): Promise<any>

}

@peter-rr try writing the module as per your needs, I am just giving an example for convert do this for @asyncapi/spec as well.

You can take some inspiration from https://github.com/asyncapi/server-api/blob/master/src/server-api.d.ts

magicmatatjahu

Please propagate my changes. There's problem with ts-node and how exactly it reads the custom typings from external modules. We should move to the Jest and then it will be easy to handle by custom jest transpiler. It's not that important if we have defined types globally or skip them, let's not make our job harder because types are not important here as we know how the package works underneath anyway (and its API).

Propagate changes and then fix tests, because all fail.

src/commands/convert.ts

Fix problem with '@asyncapi/converter' external module Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>

sonarcloud · 2022-03-07T20:51:17Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

Fix problem with '@asyncapi/specs' external module Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>

peter-rr · 2022-03-07T22:40:17Z

Please propagate my changes. There's problem with ts-node and how exactly it reads the custom typings from external modules.

Great :) Problems related to external modules are solved now and I'm trying to fix all the current failing tests. They are only 4 at this moment 💪

src/commands/convert.ts

Getting 'Command' class from 'base.ts' and not from '@oclif/core' Co-authored-by: souvik <souvikde.ns@gmail.com>

peter-rr · 2022-03-09T15:21:28Z

@Souvikns @magicmatatjahu
All tests passing now 🎉 Let me know if anything missing or could be improved :)

sonarcloud · 2022-03-11T15:04:55Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

Souvikns

Wow 🤩 looks good, wait for @magicmatatjahu's approval then we can merge this.

fmvilas

LGTM 🚀

magicmatatjahu

Thanks @peter-rr for contribution! LGTM! 🚀

magicmatatjahu · 2022-03-14T18:57:20Z

/rtm

asyncapi-bot · 2022-03-14T19:05:21Z

🎉 This PR is included in version 0.17.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

peter-rr added 5 commits January 13, 2022 22:46

Flags added to convert command

1bffea6

New try-catch block added

be8a691

Editing comments from last 'try-catch' block

1fb3d7b

Try-catch blocks unified and convert function added

4c226a9

Changes on conversion process

81808f2

peter-rr changed the title ~~Add convert command~~ feat: add new convert command to CLI Jan 20, 2022

Merge branch 'asyncapi:master' into add-convert-command

3f840d6

fmvilas requested changes Jan 21, 2022

View reviewed changes

peter-rr and others added 6 commits January 25, 2022 20:20

Merge branch 'asyncapi:master' into add-convert-command

11e041f

Update src/commands/convert.ts

a31772d

Adding `flags` when parsing `Convert` class to avoid using `Convert.flags` in the rest of the file. Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Update src/commands/convert.ts

4a0eafe

Using an object as 3rd argument for `convert()` Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Update src/commands/convert.ts

2f2a1c4

Removing `Convert.x.x` notation Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Update src/commands/convert.ts

a435fad

Removing one more `Convert.x.x` Co-authored-by: Fran Méndez <fmvilas@gmail.com>

Update src/commands/convert.ts

685d67c

Printing the content of `convertedFile` properly Co-authored-by: Fran Méndez <fmvilas@gmail.com>

peter-rr requested a review from fmvilas January 25, 2022 21:49

peter-rr and others added 6 commits January 27, 2022 13:24

Merge branch 'master' into add-convert-command

5b2f043

remove 'success message' when spec file has not been actually converted

e571953

Merge branch 'asyncapi:master' into add-convert-command

34c33a7

Merge branch 'asyncapi:master' into add-convert-command

f76547c

Assign a default value if flag 'target-version' not passed.

95bf702

Merge branch 'asyncapi:master' into add-convert-command

1153770

magicmatatjahu mentioned this pull request Feb 7, 2022

Converter should allow in-place conversion asyncapi/converter-js#93

Closed

magicmatatjahu reviewed Feb 7, 2022

View reviewed changes

src/commands/convert.ts Outdated Show resolved Hide resolved

Merge branch 'asyncapi:master' into add-convert-command

88cb0a2

Revert previous commit

401559d

peter-rr added 2 commits March 5, 2022 15:00

Add message when URL is passed.

cc3c717

Add some basic tests

db4264a

peter-rr requested a review from magicmatatjahu March 5, 2022 14:16

Souvikns reviewed Mar 7, 2022

View reviewed changes

magicmatatjahu suggested changes Mar 7, 2022

View reviewed changes

src/commands/convert.ts Show resolved Hide resolved

src/commands/convert.ts Show resolved Hide resolved

Update src/commands/convert.ts

4b7bca5

Fix problem with '@asyncapi/converter' external module Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>

peter-rr and others added 2 commits March 7, 2022 21:51

Update src/commands/convert.ts

a1942d6

Fix problem with '@asyncapi/specs' external module Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>

Fix some failing tests

bf1b672

peter-rr and others added 3 commits March 8, 2022 20:29

Change '@oclif/core' version to solve merge conflict

8e8d4a9

Merge branch 'asyncapi:master' into add-convert-command

ae36f4e

Fix more failing tests

2a912bf

Souvikns reviewed Mar 9, 2022

View reviewed changes

src/commands/convert.ts Outdated Show resolved Hide resolved

Update src/commands/convert.ts

afa2cdc

Getting 'Command' class from 'base.ts' and not from '@oclif/core' Co-authored-by: souvik <souvikde.ns@gmail.com>

Merge branch 'master' into add-convert-command

fd58222

Souvikns approved these changes Mar 14, 2022

View reviewed changes

fmvilas approved these changes Mar 14, 2022

View reviewed changes

magicmatatjahu approved these changes Mar 14, 2022

View reviewed changes

asyncapi-bot added the ready-to-merge label Mar 14, 2022

asyncapi-bot merged commit 61f8da7 into asyncapi:master Mar 14, 2022

asyncapi-bot added the released label Mar 14, 2022

fmvilas mentioned this pull request Apr 29, 2022

Add convert command #41

Closed

peter-rr deleted the add-convert-command branch July 20, 2022 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add new `convert` command to CLI #188

feat: add new `convert` command to CLI #188

peter-rr commented Jan 20, 2022 •

edited

Loading

fmvilas left a comment

peter-rr commented Jan 25, 2022

peter-rr commented Feb 3, 2022

peter-rr commented Feb 3, 2022 •

edited

Loading

Souvikns commented Feb 4, 2022

peter-rr commented Feb 4, 2022 •

edited

Loading

Souvikns commented Feb 5, 2022

magicmatatjahu left a comment

peter-rr commented Mar 4, 2022

Souvikns Mar 7, 2022

magicmatatjahu left a comment

sonarcloud bot commented Mar 7, 2022

peter-rr commented Mar 7, 2022 •

edited

Loading

peter-rr commented Mar 9, 2022

sonarcloud bot commented Mar 11, 2022

Souvikns left a comment

fmvilas left a comment

magicmatatjahu left a comment

magicmatatjahu commented Mar 14, 2022

asyncapi-bot commented Mar 14, 2022

-declare module '@asyncapi/converter';
+declare module '@asyncapi/converter' {
+  export async function convert(spec: string, targetVersion: string, options: any): Promise<any>
+}

feat: add new convert command to CLI #188

feat: add new convert command to CLI #188

Conversation

peter-rr commented Jan 20, 2022 • edited Loading

fmvilas left a comment

Choose a reason for hiding this comment

peter-rr commented Jan 25, 2022

peter-rr commented Feb 3, 2022

peter-rr commented Feb 3, 2022 • edited Loading

Souvikns commented Feb 4, 2022

peter-rr commented Feb 4, 2022 • edited Loading

Souvikns commented Feb 5, 2022

magicmatatjahu left a comment

Choose a reason for hiding this comment

peter-rr commented Mar 4, 2022

Souvikns Mar 7, 2022

Choose a reason for hiding this comment

magicmatatjahu left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Mar 7, 2022

peter-rr commented Mar 7, 2022 • edited Loading

peter-rr commented Mar 9, 2022

sonarcloud bot commented Mar 11, 2022

Souvikns left a comment

Choose a reason for hiding this comment

fmvilas left a comment

Choose a reason for hiding this comment

magicmatatjahu left a comment

Choose a reason for hiding this comment

magicmatatjahu commented Mar 14, 2022

asyncapi-bot commented Mar 14, 2022

feat: add new `convert` command to CLI #188

feat: add new `convert` command to CLI #188

peter-rr commented Jan 20, 2022 •

edited

Loading

peter-rr commented Feb 3, 2022 •

edited

Loading

peter-rr commented Feb 4, 2022 •

edited

Loading

peter-rr commented Mar 7, 2022 •

edited

Loading