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 Components, Security Requirement(s) models and other improvements #71

Merged
merged 29 commits into from
Feb 8, 2023

Conversation

smoya
Copy link
Member

@smoya smoya commented Aug 25, 2022

Description

Some tooling need to access components directly to components so the model is required. Also added CorrelationIds as it was needed by the Components model.
Examples of why this is needed:

This PR also adds the models for SecurityRequirement, MessageExample and the collection of those, SecurityRequirements and MessageExamples.
It also includes some fixes on types.

@smoya smoya changed the title feat: add 'components' and 'correlationIds' models feat: add 'components' model Aug 26, 2022
docs/v1.md Outdated Show resolved Hide resolved
@smoya smoya requested a review from magicmatatjahu August 26, 2022 09:02
Copy link
Member

@magicmatatjahu magicmatatjahu left a comment

Choose a reason for hiding this comment

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

Last comment 😄

docs/v1.md Outdated Show resolved Hide resolved
magicmatatjahu
magicmatatjahu previously approved these changes Aug 26, 2022
Copy link
Member

@magicmatatjahu magicmatatjahu 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
fmvilas previously approved these changes Aug 26, 2022
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 👍

@smoya
Copy link
Member Author

smoya commented Aug 29, 2022

@fmvilas @magicmatatjahu I added the operations() method to Components as I forgot to add it.

docs/v1.md Outdated Show resolved Hide resolved
magicmatatjahu
magicmatatjahu previously approved these changes Aug 29, 2022
@smoya smoya changed the title feat: add 'components' model feat: add Components, and Security Requirement(s) models Aug 29, 2022
@smoya smoya requested a review from magicmatatjahu August 29, 2022 13:51
@smoya
Copy link
Member Author

smoya commented Aug 29, 2022

I decided to include more things into this PR so we do the review at once.

I just added:

models for SecurityRequirement and the collection of those, SecurityRequirements.
It also includes minor fixes on types.

cc @magicmatatjahu @jonaslagoni

docs/v1.md Outdated Show resolved Hide resolved
@smoya smoya requested a review from magicmatatjahu October 10, 2022 11:27
docs/v1.md Outdated Show resolved Hide resolved
Co-authored-by: Maciej Urbańczyk <urbanczyk.maciej.95@gmail.com>
@smoya
Copy link
Member Author

smoya commented Oct 27, 2022

According to asyncapi/parser-js#663 (comment), version() from Extension has been removed.

@fmvilas
Copy link
Member

fmvilas commented Nov 14, 2022

@magicmatatjahu can you please take care of this PR while Sergio is out?

@magicmatatjahu
Copy link
Member

@fmvilas No problem, I'm up-to-date with it :)

@fmvilas
Copy link
Member

fmvilas commented Dec 23, 2022

What's missing here, @magicmatatjahu? Can we already merge it or are you still working on it?

@smoya
Copy link
Member Author

smoya commented Feb 2, 2023

Is there something blocking this PR to be merged? Cc @magicmatatjahu @jonaslagoni

Otherwise, i feel we can proceed.

@magicmatatjahu
Copy link
Member

@smoya New things from 3.0.0 like server'shost, pathName, operation's reply object etc. I will have new things in this PR asyncapi/parser-js#708 but I need propagate them in this PR.

@smoya
Copy link
Member Author

smoya commented Feb 2, 2023

@smoya New things from 3.0.0 like server'shost, pathName, operation's reply object etc. I will have new things in this PR asyncapi/parser-js#708 but I need propagate them in this PR.

I would rather merge this one, which is being old enough and with tons of changes and then work on top of it. otherwise this PR is growing and lacking context with every day it passes.

@smoya
Copy link
Member Author

smoya commented Feb 8, 2023

@smoya New things from 3.0.0 like server'shost, pathName, operation's reply object etc. I will have new things in this PR asyncapi/parser-js#708 but I need propagate them in this PR.

I would rather merge this one, which is being old enough and with tons of changes and then work on top of it. otherwise this PR is growing and lacking context with every day it passes.

wdyt about this @jonaslagoni @magicmatatjahu @derberg @fmvilas ?

@fmvilas
Copy link
Member

fmvilas commented Feb 8, 2023

Whatever works best for you guys. Just let's make sure to have this (and the parser) as soon as possible.

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Feb 8, 2023

Ok I think we can merge it :) I fixed all bugs that I found. I removed some not implemented methods like extension(name: string) => Extension - it won't be a breaking change when we will add that method in e.g. 2.1.0.

In next PR I will add methods/objects related to the 3.0.0 version

cc @smoya @jonaslagoni Could you accept it?

@magicmatatjahu
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 31759dc into asyncapi:master Feb 8, 2023
@magicmatatjahu magicmatatjahu deleted the feat/components branch February 8, 2023 14:25
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

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.

5 participants