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: adapt Kafka bindings to v3 #221

Closed

Conversation

lbroudoux
Copy link
Collaborator

Signed-off-by: Laurent Broudoux laurent.broudoux@gmail.com

Description

As discussed and proposed here in issue #182, this PR:

  • Adapts README to explain with have different versions of bindings for v3 and v3,
  • Add specific README to track v2 related changes and documentation,
  • Add specific README to track v2 related changes and documentation.

Another PR with specific schemas for v2 and v3 is on its way on asyncapi/spec-json-schemas.

Related issue(s)

Relates to #182

Signed-off-by: Laurent Broudoux <laurent.broudoux@gmail.com>
dalelane
dalelane previously approved these changes Nov 12, 2023
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.

spotted a few minor things we could fix, but overall this is exactly what we need

kafka/README-v2.md Outdated Show resolved Hide resolved
kafka/README-v2.md Outdated Show resolved Hide resolved
kafka/README-v3.md Outdated Show resolved Hide resolved
kafka/README-v3.md Outdated Show resolved Hide resolved
Co-authored-by: Dale Lane <dale.lane@gmail.com>
lbroudoux and others added 2 commits November 13, 2023 15:57
Co-authored-by: Dale Lane <dale.lane@gmail.com>
Co-authored-by: Dale Lane <dale.lane@gmail.com>
@lbroudoux
Copy link
Collaborator Author

All good: fix the typos and apply suggestions. Thanks @dalelane, for the review.

dalelane
dalelane previously approved these changes Nov 13, 2023
@derberg
Copy link
Member

derberg commented Nov 14, 2023

just make sure you target next-major-spec branch 🙏🏼 and then merge

@derberg
Copy link
Member

derberg commented Nov 14, 2023

oh that is intentional? but not much aligned with others, eventually master should contain latest and if you want maybe we can on release date create v2 tag here in this repo, so you can link to it?

cc @jonaslagoni

@smoya
Copy link
Member

smoya commented Nov 20, 2023

oh that is intentional? but not much aligned with others, eventually master should contain latest

Both solutions are good enough to me 🤔. @derberg's solution ends up looking cleaner for maintainers. @lbroudoux suggestion seems kinda clearer for users when reaching this repo. At least for now, not sure when more spec major versions start going out.

and if you want maybe we can on release date create v2 tag here in this repo, so you can link to it?

Additionally, we will need to change the links in v2 spec that point to this repo so they point to bindings compatible with spec v2, otherwise, people will reach bindings for spec v3 instead.
As a technical detail, we might wanna use a branch called v2 rather than a tag, so in case we need to release a fix for any binding for spec v2, we can do it in such branch and we won't need to retag.

@lbroudoux lbroudoux changed the base branch from master to next-major-spec November 24, 2023 16:28
@lbroudoux lbroudoux dismissed dalelane’s stale review November 24, 2023 16:28

The base branch was changed.

@lbroudoux
Copy link
Collaborator Author

lbroudoux commented Nov 24, 2023

Now replaced by #226.

Could you all confirm we can cancel this one?

@dalelane
Copy link
Collaborator

agreed

@dalelane dalelane closed this Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants