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

Road to v2.0.0 #481

Closed
14 of 20 tasks
magicmatatjahu opened this issue Mar 3, 2022 · 35 comments
Closed
14 of 20 tasks

Road to v2.0.0 #481

magicmatatjahu opened this issue Mar 3, 2022 · 35 comments
Labels
enhancement New feature or request keep-open

Comments

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Mar 3, 2022

This issue contains list of possible features that we wanna introduce in the v2.0.0 of ParserJS. Have in mind that some of them may not be implemented. You can also write your own proposals/features/PRs in the comments, I will update then the main list.

Note: Mainly PRs should be merged to the next-major branch. Additionally, some of the following issues will only be fixable/implementable once the other issues are resolved.

Nice To Have

cc @jonaslagoni @smoya

@smoya
Copy link
Member

smoya commented Mar 7, 2022

@magicmatatjahu would you mind adding #482 into the list?

@magicmatatjahu
Copy link
Member Author

@smoya Added :)

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@magicmatatjahu as #294 is not really a BC, we can remove it from the list so we move forward just with the minimum.

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@magicmatatjahu as #405 is not really a BC, we can remove it from the list so we move forward just with the minimum.

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@magicmatatjahu as #404 is not really a BC, we can remove it from the list so we move forward just with the minimum.

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@magicmatatjahu I'm concerned about #403 as this would become a BC, so perfect for the new parser version, but it requires a lot of effort. Do you think we should invest on this for the next version of the Parser? cc @jonaslagoni @fmvilas

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@magicmatatjahu can you annotate #201 is done?

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@magicmatatjahu as #157 is not really a BC, we can remove it from the list so we move forward just with the minimum.

@smoya
Copy link
Member

smoya commented Jul 11, 2022

We could build another list, instead of removing issues, with the Nice To Have issues. In case there is time or people willing to take those.

EDIT: Here is the list @magicmatatjahu . You can add it right after the current list on the description of this issue.

Nice To Have

@smoya
Copy link
Member

smoya commented Jul 11, 2022

@magicmatatjahu This is going to be needed as well asyncapi/parser-api#69

@smoya
Copy link
Member

smoya commented Jul 26, 2022

After giving another thought to the list of tasks for v2.0.0 of the parser, I think we can go further and reduce the scope. I considered AsyncAPI Roadmap at all moments to see how we could deliver value asap (the new API).

2022-07-26 at 20 47 12@2x

Here is what I would suggest we could move with, but I know @magicmatatjahu had strong reasons not to discard some of the tasks, so I'm going to ask you if you could expand info on why you believe those are needed. I'm addressing to you since you created most of those issues, also because you have a good knowledge of the v1.x code base, however, anyone is invited and encouraged to jump into this discussion so we can find the minimum required features for the new version.

@jonaslagoni
Copy link
Sponsor Member

Tbh, the only thing I see as required would be #482 (because that's already underway) and #401 (because well, that is what triggers this rewrite).

My reason for only those two is that is what is directly needed from the generator's perspective and probably where #401 is needed the most.

That said, remember... You can do another major release in another month if that's what you would like to do where you address even more of these issues. Nobody is stopping you from releasing new major versions of the parser.

@magicmatatjahu
Copy link
Member Author

@smoya

#477 -> Ok, it's a really cool feature from the developer point of view, but could we copy the current parser-js logic into the new TS version?

I already explained you. For me it would be too much pointless work, because from the beginning there was a thought to use Spectral and for that we have collaborations with Spotlight team regarding the official ruleset AsyncAPI. However if community thinks that it's better idea (it's usually right, isn't?) we can do this, but we should discuss that. cc @derberg @jonaslagoni @fmvilas

#480 -> Is it really needed for this version? If we don't move into spectral atm, I think it's not.

If we do not go with Spectral then it is not needed, but the function validate is made to use it later in the context of Spectral and to return errors when validating the used schema in a custom format, that is, where the problem occurred, etc. Currently, the given solution does not have this, and for me this is a serious problem from the UX level - the user does not know where the error is and sometimes also what problem, because he/she see only runtime error, not meaningful error for human,

#119 -> we can create a common interface for those errors. Once we integrate spectral, those could be later on be adapted to our own interface so we won't introduce any breaking change.

For this we have in mentorship program that project https://github.com/imabp/asyncapi_problem but with Spectral we won't need that in 99% cases. As I think there, even in the case of using logic from our parser, it should not be necessary. I would throw this issue from the scope and even close it.

@smoya
Copy link
Member

smoya commented Jul 27, 2022

I already explained you.

Yeah, but we talked privately so I wanted to add visibility to this, opening the discussion to whoever is interested in this whole community.

because from the beginning there was a thought to use Spectral and for that we have collaborations with Spotlight team regarding the official ruleset AsyncAPI.

I understand moving to Spotlight is something we must eventually do. The fact we unify validation logic into just one place, the way Spectral does validation based on rules, etc, is way better to what we have right now in current parser.
However, by reading #477, and even being aware of the work @jonaslagoni and @magicmatatjahu did in Spectral, I can't see we agreed anywhere to become part of v3.0 as a must.

On that side, I agree @jonaslagoni on the following statement:

You can do another major release in another month if that's what you would like to do where you address even more of these issues. Nobody is stopping you from releasing new major versions of the parser.

Having said that, I would love to know what is the status of the development of AsyncAPI rules on Spectral. Is there any rule that the current parser has but Spectral doesn't? Maybe there is an issue tracking the progress that I'm missing (sorry if that's the case).

I'm asking that because if we are done in Spectral side, maybe we could evaluate moving forward with only the following in addition to what I mentioned:

Thoughts? Ideas?

@smoya
Copy link
Member

smoya commented Sep 20, 2022

@magicmatatjahu could u please add #620 to the list of TODO?

@smoya
Copy link
Member

smoya commented Sep 20, 2022

Also this one @magicmatatjahu asyncapi/parser-api#69

@magicmatatjahu
Copy link
Member Author

@smoya Done.

@fmvilas
Copy link
Member

fmvilas commented Mar 29, 2023

What's really stopping us from releasing Parser v2? I want to move this forward as soon as possible so we can start receiving feedback.

@smoya
Copy link
Member

smoya commented Mar 29, 2023

What's really stopping us from releasing Parser v2? I want to move this forward as soon as possible so we can start receiving feedback.

There is missing work that should go in Spec v3.0.0 that still is not supported in the Parser. Champions are willing to work on it. See asyncapi/spec#691 (comment)

Some kind of summary:

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

No branches or pull requests

5 participants