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 multiparser to the parserjs monorepo repository. #1036

Merged
merged 118 commits into from
Jul 23, 2024
Merged

feat: add multiparser to the parserjs monorepo repository. #1036

merged 118 commits into from
Jul 23, 2024

Conversation

ayushnau
Copy link
Contributor

@ayushnau ayushnau commented Jul 4, 2024

Changes Added.

Added the Multiparser to the turborepo. 
Added the required scripts of the turborepo to the root package.json file.

Related issue(s)

Related to: #963

smoya and others added 30 commits August 18, 2023 12:52
feat: export types so any other libraries can use them
* update dependencies and add support for v3
@asyncapi-bot asyncapi-bot added the bounty AsyncAPI Bounty label Jul 5, 2024
"@asyncapi/protobuf-schema-parser": "^3.0.0",
"@asyncapi/raml-dt-schema-parser": "^4.0.4",
"parserapiv1": "npm:@asyncapi/parser@^2.1.0",
"parserapiv2": "npm:@asyncapi/[email protected]"
Copy link
Member

Choose a reason for hiding this comment

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

Where is the parserapiv3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes this is shifted to line number 65. as "@asyncapi/parser": "file:../parser"
which will take the latest version as a @asyncapi/parser

Copy link
Member

Choose a reason for hiding this comment

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

But why devDependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I was thinking as the parser is in the local now will not need to keep it as dependency. but now i am thinking though the test are passing successfully it should be in dependency only. anyway updated the added it back.

@ayushnau
Copy link
Contributor Author

ayushnau commented Jul 8, 2024

image

@ayushnau
Copy link
Contributor Author

ayushnau commented Jul 8, 2024

@smoya please can you merge this.

@smoya
Copy link
Member

smoya commented Jul 8, 2024

@smoya can you merge this. now.

LOL i hope it is not an order 😂. Will review and merge when i have a chance. I'm out on holidays, and sick at the same time.

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.

First review round

turbo.json Outdated
"build": {}
"test": {"cache": false},
"prepublishOnly": {"cache": false},
"test:unit": {"cache": false},
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Does this file have a wrong indentation now?

Copy link
Member

Choose a reason for hiding this comment

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

ping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the correct indentation.

"@asyncapi/raml-dt-schema-parser": "^4.0.4",
"parserapiv1": "npm:@asyncapi/parser@^2.1.0",
"parserapiv2": "npm:@asyncapi/[email protected]",
"@asyncapi/parser": "file:../parser"
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason for changing the dependency name? If there isnt, please do not modify the original code from https://github.com/smoya/multi-parser-js/blob/19750c914386d4417c9e17b13584634132805e27/package.json#L47

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, we used parserv3, but now that the repositories are merged and multiparser is using the latest version, it makes sense to refer to it by the correct name. If anyone needs to know the version, they can simply check the version of parser.

Copy link
Member

Choose a reason for hiding this comment

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

The reason the version was explicit is exactly because we wanted it to be explicit for each version

Copy link
Member

Choose a reason for hiding this comment

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

ping @ayushnau

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok fixed this.

@smoya
Copy link
Member

smoya commented Jul 12, 2024

How would new releases of the @asyncapi/parser affect this multi-parser package?
How shall we do releases now if we want to sync the dependency of the @asyncapi/parser in @asyncapi/multi-parser ?

@ayushnau
Copy link
Contributor Author

ayushnau commented Jul 12, 2024

How would new releases of the @asyncapi/parser affect this multi-parser package? How shall we do releases now if we want to sync the dependency of the @asyncapi/parser in @asyncapi/multi-parser

  1. Now whenever @asyncapi/parser will be updated. New md file will be created in the .changeset folder stating the package and the level of change.
  2. Now as the @asyncapi/multiparser is the directly using the local @asyncapi/parser. The dependency for the multiparser will be updated.
  3. Now while creating the md I am suggesting to add the @asyncapi/multiparser along with the parser, as the dependency for multiparser is also changed. both package will be release together.
---
"@asyncapi/parser": minor
"@asyncapi/multi-parser": patch


changes.

yes that would mean we dont need to do anything else to sync the parser with multiparser it will be automatically synced

@smoya

@smoya
Copy link
Member

smoya commented Jul 15, 2024

How would new releases of the @asyncapi/parser affect this multi-parser package? How shall we do releases now if we want to sync the dependency of the @asyncapi/parser in @asyncapi/multi-parser

  1. Now whenever @asyncapi/parser will be updated. New md file will be created in the .changeset folder stating the package and the level of change.
  2. Now as the @asyncapi/multiparser is the directly using the local @asyncapi/parser. The dependency for the multiparser will be updated.
  3. Now while creating the md I am suggesting to add the @asyncapi/multiparser along with the parser, as the dependency for multiparser is also changed. both package will be release together.
---
"@asyncapi/parser": minor
"@asyncapi/multi-parser": patch


changes.

yes that would mean we dont need to do anything else to sync the parser with multiparser it will be automatically synced

@smoya

I guess it works by now. We could work on some automation in the future for this. 👍

@ayushnau
Copy link
Contributor Author

will be giving update on this by on weekend. currently cant take it.

@ayushnau
Copy link
Contributor Author

@smoya updated this please check.

turbo.json Show resolved Hide resolved
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! 🚀🌔

@smoya
Copy link
Member

smoya commented Jul 23, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit 4556aec into asyncapi:master Jul 23, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

8 participants