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: make server variables reuseable #174

Conversation

danielkocot
Copy link

Description

Part of asyncapi/spec#717

This PR adds serverVariables as reusable objects to be used in components.

Related issue(s)
asyncapi/spec#717

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 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
0.0% 0.0% Duplication

@smoya
Copy link
Member

smoya commented Feb 16, 2022

For the record, here is the diff between 2.3.0.json and 2.4.0.json (introduced here):

➜  diff -u <(curl -s https://raw.githubusercontent.com/asyncapi/spec-json-schemas/master/schemas/2.3.0.json) <(curl -s https://raw.githubusercontent.com/asyncapi/spec-json-schemas/0af02c5b2799ae0ea23bb07e13e57d95d5a1dc60/schemas/2.4.0.json)
--- /dev/fd/11  2022-02-16 12:55:18.730955251 +0100
+++ /dev/fd/12  2022-02-16 12:55:18.731524237 +0100
@@ -1,5 +1,5 @@
 {
-  "title": "AsyncAPI 2.3.0 schema.",
+  "title": "AsyncAPI 2.4.0 schema.",
   "$schema": "http://json-schema.org/draft-07/schema#",
   "type": "object",
   "required": [
@@ -17,7 +17,7 @@
     "asyncapi": {
       "type": "string",
       "enum": [
-        "2.3.0"
+        "2.4.0"
       ],
       "description": "The AsyncAPI specification version of this document."
     },
@@ -270,6 +270,9 @@
         "servers": {
           "$ref": "#/definitions/servers"
         },
+        "serverVariables": {
+          "$ref": "#/definitions/serverVariables"
+        },
         "channels": {
           "$ref": "#/definitions/channels"
         },

Copy link
Member

@smoya smoya left a comment

Choose a reason for hiding this comment

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

LGTM, However I'm not a Code Owner.

I think we would need to create a new release branch for the next minor and point this PR to it.

Any thoughts? cc @fmvilas @derberg

Copy link
Member

@fmvilas fmvilas left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@fmvilas
Copy link
Member

fmvilas commented Feb 17, 2022

/dnm

@fmvilas
Copy link
Member

fmvilas commented Feb 17, 2022

Let's wait for @derberg and @dalelane review too.

@fmvilas fmvilas requested review from dalelane and derberg February 17, 2022 19:45
Copy link
Collaborator

@dalelane dalelane left a comment

Choose a reason for hiding this comment

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

I think @smoya is right that we can't merge this into master as-is so we need to either park it for a bit, or switch the PR to target somewhere ready for the next release.

But those git-logistics aside, this looks like a good idea to me

@derberg
Copy link
Member

derberg commented Mar 9, 2022

we need to find volunteer to run 2.4.0 release coordination 😓

@smoya
Copy link
Member

smoya commented Mar 9, 2022

@derberg we need a next branch so we can merge the code that will be released in 2.4.0.

@smoya
Copy link
Member

smoya commented Mar 9, 2022

we need to find volunteer to run 2.4.0 release coordination 😓

I volunteer 👍

@derberg
Copy link
Member

derberg commented Mar 10, 2022

I volunteer 👍

oh, that was easy ❤️

branches created:

I highly recommend you start coordinating by opening an issue just like proposed by @dalelane in this PR and also using his updated release process instructions. I left few comments that but majority of the content is what should be followed

@smoya
Copy link
Member

smoya commented Mar 10, 2022

I volunteer 👍

oh, that was easy ❤️

branches created:

I highly recommend you start coordinating by opening an issue just like proposed by @dalelane in this PR and also using his updated release process instructions. I left few comments that but majority of the content is what should be followed

I thought we would finally follow semantic-release and instead use next branches (as we do for next-major) for those releases.

I suggested it in the past in Slack as well: https://asyncapi.slack.com/archives/C0230UAM6R3/p1644418043568169

@derberg
Copy link
Member

derberg commented Mar 14, 2022

I thought we would finally follow semantic-release and instead use next branches (as we do for next-major) for those releases.
I suggested it in the past in Slack as well: https://asyncapi.slack.com/archives/C0230UAM6R3/p1644418043568169

well I remember the discussion about next-major but do not recall being involved in the discussion about spec release branches that are named after release cadence. I personally like the current names of release branches, they clearly indicate what you can expect + they make sense as you know when you release, while with next you clearly not. Anyway, if you want to change it you need to start open discussion as for now it is part of https://github.com/asyncapi/spec/blob/master/RELEASE_PROCESS.md and was agreed in a discussion about release cadence asyncapi/spec#513. Just clear explain what are the benefits.

@smoya
Copy link
Member

smoya commented Mar 14, 2022

Anyway, if you want to change it you need to start open discussion as for now it is part of https://github.com/asyncapi/spec/blob/master/RELEASE_PROCESS.md and was agreed in a discussion about release cadence asyncapi/spec#513. Just clear explain what are the benefits.

I ended up creating such issue asyncapi/spec#734! Thanks @derberg for your suggestion!

@smoya
Copy link
Member

smoya commented Mar 15, 2022

@danielkocot we have now the release branches for 2.4.0. Would you mind changing the base branch to 2022-04-release ? Thanks!

EDIT: NVM! better to wait first #182 to be merged. Then I will ping you again. Thanks.

@smoya smoya mentioned this pull request Mar 21, 2022
55 tasks
@danielkocot danielkocot changed the base branch from master to 2022-04-release March 21, 2022 19:27
@derberg
Copy link
Member

derberg commented Apr 5, 2022

@smoya we still have it scheduled for April right?

@smoya
Copy link
Member

smoya commented Apr 5, 2022

@smoya we still have it scheduled for April right?

Yes, we do. asyncapi/spec#735

@smoya
Copy link
Member

smoya commented Apr 5, 2022

@danielkocot would you mind updating your branch with latest upstream changes? It seems there are some conflicts.

@fmvilas
Copy link
Member

fmvilas commented Apr 13, 2022

I'm seeing a lot of changes in Markdown files. Is that expected? 🤔

@smoya
Copy link
Member

smoya commented Apr 13, 2022

/autoupdate

@smoya
Copy link
Member

smoya commented Apr 13, 2022

I'm seeing a lot of changes in Markdown files. Is that expected? 🤔

I think for some reason, target branch didn't get updated with master branch changes, and those changes introduced in 7e9a2a3 are not in there.
Will see what is happening.

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.

all good but I see some conflicts there that will block merging

@smoya
Copy link
Member

smoya commented Apr 20, 2022

@danielkocot We are almost there! Only one thing pending: would you mind fixing the merge conflicts? After you do it, I will merge it.

@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

@smoya
Copy link
Member

smoya commented Apr 21, 2022

/rtm

@asyncapi-bot asyncapi-bot merged commit c10567f into asyncapi:2022-04-release Apr 21, 2022
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 2.14.0-2022-04-release.2 🎉

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants