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

Update to contribution guide - make converter and examples must have for release #960

Closed
derberg opened this issue Aug 8, 2023 · 4 comments

Comments

@derberg
Copy link
Member

derberg commented Aug 8, 2023

Discussion started here -> https://youtu.be/WW5sLVHa0b8?t=474

tl;dr currently guide says that for the release we also need:

- Compliant implementation in the AsyncAPI JS Parser and the AsyncAPI JSON Schema

I want to suggest we extend it with:

- Compliant implementation in the AsyncAPI JS Parser, the AsyncAPI JSON Schema and the AsyncAPI Converter
- Examples converted to latest version

Why converter

  • we anyway always make converter ready for the release as later we use it in the Studio to enable people to convert quickly to latest
  • for minor releases conversion in huge majority of cases is just asyncapi value change, as in the end minors are non breaking, not much to convert (@magicmatatjahu please correct me if I'm wrong)
  • @jonaslagoni already provided a PR with a script in the spec repo to use converter to convert examples to latest. If we make converter must have, releasing and examples conversion will be easy process

Why examples

  • be consistent, in master we keep latest only. But now we keep latest spec, but examples are from older versions
  • if somebody want examples from older versions, we just need to make sure readme have links to old tags, that is it

What could possibly go wrong. In theory we are not really slowing down the release process.

Thoughts?

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Aug 8, 2023

With the examples I agree because there were problems with the release of 2.4.0 or 2.5.0 (I don't remember) that they were not updated.

for minor releases conversion in huge majority of cases is just asyncapi value change, as in the end minors are non breaking, not much to convert (@magicmatatjahu please correct me if I'm wrong)

Yes, for minor versions converter only bumps asyncapi version.

@jonaslagoni already provided a PR with a script in the spec repo to use converter to convert examples to latest. If we make converter must have, releasing and examples conversion will be easy process

I suggest to write examples with 3.0.0 version from scratch because converter some things, which doesn't know, like ids for channels or operations, generates automatically and their ids are not good looking - another "problems" are described there https://github.com/asyncapi/converter-js#conversion-2xx-to-3xx The converter is ok but the examples should be "nice" for new users.

NOTE: But for minor release we can use converter, like for 3.1.0 etc. we can use converter :)

@jonaslagoni
Copy link
Member

I want to suggest we extend it with:

  • Compliant implementation in the AsyncAPI JS Parser, the AsyncAPI JSON Schema and the AsyncAPI Converter
  • Examples converted to latest version

I agree 👍

As a side note, I think we have to enforce that any repository that is required to update on spec releases, MUST have the same codeowners (or at least the very minimum spec owners), so we ensure that there is never a conflict of interest.

I suggest to write examples with 3.0.0 version from scratch because converter some things, which doesn't know, like ids for channels or operations, generates automatically and their ids are not good looking - another "problems" are described there https://github.com/asyncapi/converter-js#conversion-2xx-to-3xx The converter is ok but the examples should be "nice" for new users.

@magicmatatjahu I disagree entirely with manually updating existing examples from scratch 😆

If the converter out of the box does not help with this problem, then it's missing some serious love. It's the whole reason we have the library 😆 I get there are scenarios it cannot handle and there it's completely fine to manually change the examples after conversion, but the majority should be automated 😄

@smoya
Copy link
Member

smoya commented Sep 12, 2023

I want to suggest we extend it with:

- Compliant implementation in the AsyncAPI JS Parser, the AsyncAPI JSON Schema and the AsyncAPI Converter
- Examples converted to latest version

I agree and I would go further and include Parser-API, if any change is needed. Important to include it in whenever place we mention the Parser-JS.

If the converter out of the box does not help with this problem, then it's missing some serious love. It's the whole reason we have the library 😆 I get there are scenarios it cannot handle and there it's completely fine to manually change the examples after conversion, but the majority should be automated

Just mix both suggestions. First use the converter, then fine tune the result so it looks better (in terms of generated Id's, etc), specially for new users.

Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 11, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants