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: add convert function from 2.0.0 to 2.1.0 #44

Merged
merged 11 commits into from
Jun 29, 2021

Conversation

magicmatatjahu
Copy link
Member

@magicmatatjahu magicmatatjahu commented Jun 24, 2021

Description

  • add convert function from 2.0.0 to 2.1.0

  • change the logic for convert the spec: in PR we convert the spec from all specs between input and output version, it means that for 1.2.0 to 2.1.0 we convert spec by functions which convert:

    • 1.2.0 -> 2.0.0-rc1
    • 2.0.0-rc1 -> 2.0.0-rc2
    • 2.0.0-rc2 -> 2.0.0
    • 2.0.0 -> 2.1.0

    By this we don't need logic to write function something like from2rc1to2_1_0 etc...

  • added tests

Related issue(s)
Resolves #42

@magicmatatjahu magicmatatjahu added the enhancement New feature or request label Jun 24, 2021
Copy link

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

@magicmatatjahu magicmatatjahu changed the title Support 2.1.0 feat: add convert function from 2.0.0 to 2.1.0 Jun 24, 2021
Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

I am wondering, is it not enough that we test with streetlight?

Anything extra should be covered by unit tests.

@magicmatatjahu
Copy link
Member Author

I am wondering, is it not enough that we test with streetlight?

Anything extra should be covered by unit tests.

What for example? 😄 We bump only version of spec between 2.0.0 -> 2.1.0

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

as @jonaslagoni is looking into code already, I'm here to just remind you that once we will merge this one, we need to bump it right away in the playground. We also need to discuss if we should offer automated convert from 2.0 to 2.1 as we do with 1->2

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

Think it is enough to reuse Streetlight and not the other two, as we already have them as inputs in 1.2.0 🤔

test/input/2.0.0/gitter-streaming.yml Outdated Show resolved Hide resolved
test/input/2.0.0/slack-rtm.yml Outdated Show resolved Hide resolved
@magicmatatjahu
Copy link
Member Author

@derberg As I wrote in other places, 2.1.0 has only additions so we can convert automatically :)

@jonaslagoni
Copy link
Member

jonaslagoni commented Jun 25, 2021

we need to bump it right away in the playground

Wouldn't the CI system take care of that? Or is it not enabled for the playground? 🤔

@derberg
Copy link
Member

derberg commented Jun 25, 2021

@jonaslagoni bump is not enough. Additional changes are needed to enable or disable 2.0->2.1 conversion

As I wrote in other places, 2.1.0 has only additions so we can convert automatically :)

@magicmatatjahu thanks for pointing it out, I had no idea. We can convert automatically from 1 to 2 version too. The point is not what we can do technically but what we should do from UX perspective. We need to talk and agree if we do similar modal to playground.asyncapi.io/?url=https://raw.githubusercontent.com/asyncapi/spec/master/examples/1.2.0/slack-rtm.yml or not. Good UX would be to provide such modal, with optional conversion and link to release notes, but this is just my opinion that needs to be validated. Will you drive it forward or should I?

jonaslagoni
jonaslagoni previously approved these changes Jun 25, 2021
Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@magicmatatjahu
Copy link
Member Author

@derberg I can handle that :)

lib/index.js Outdated Show resolved Hide resolved
@magicmatatjahu magicmatatjahu requested a review from derberg June 29, 2021 10:55
@sonarqubecloud
Copy link

Kudos, SonarCloud 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
0.0% 0.0% Duplication

@magicmatatjahu magicmatatjahu merged commit 24457c4 into asyncapi:master Jun 29, 2021
@magicmatatjahu magicmatatjahu deleted the support-2.1.0 branch June 29, 2021 10:58
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support version 2.1.0
4 participants