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: introduce the turborepo to the parser.js #992

Merged
merged 24 commits into from
Jun 7, 2024
Merged

feat: introduce the turborepo to the parser.js #992

merged 24 commits into from
Jun 7, 2024

Conversation

ayushnau
Copy link
Contributor

@ayushnau ayushnau commented Apr 21, 2024

Changes Added.

Turborepo Added.
Converted the parser repo to app, inside the turborepo.
Added the required scripts of the turborepo to the root package.json file.

Related issue(s)

Related to: #963

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@ayushnau
Copy link
Contributor Author

Deliverables:

  1. Integrate turborepo in parser.js
  2. Integrate multiparser in the parser.js.

@ayushnau
Copy link
Contributor Author

@smoya please review the pr.

@ayushnau ayushnau changed the title feat: Introduce the turborepo to the parser.js feat: introduce the turborepo to the parser.js Apr 21, 2024
@smoya
Copy link
Member

smoya commented Apr 21, 2024

/dnm

@smoya
Copy link
Member

smoya commented Apr 21, 2024

Blocking the merge until we review it

README.md Outdated
@@ -1,533 +1,81 @@
[![AsyncAPI JavaScript Parser](./assets/logo.png)](https://www.asyncapi.com)
# Turborepo starter
Copy link
Member

Choose a reason for hiding this comment

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

We need a proper README. This is just a placeholder.

Choose a reason for hiding this comment

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

added the readme.

apps/parser/CODEOWNERS Outdated Show resolved Hide resolved
apps/parser/CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

This file should go in the root of the repository

Choose a reason for hiding this comment

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

added in root.

Copy link
Member

Choose a reason for hiding this comment

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

You didn't move it but duplicated

apps/parser/.gitignore Outdated Show resolved Hide resolved
turbo.json Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@ayushnau
Copy link
Contributor Author

ayushnau commented Jun 1, 2024

@smoya It is working fine now. I changed the version and now it is working.
image

smoya
smoya previously approved these changes Jun 2, 2024
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 Jun 2, 2024

@ayushnau There is an error, seems a not updated .lock file or similar https://github.com/asyncapi/parser-js/actions/runs/9291475481/job/25618079508?pr=992

@ayushnau
Copy link
Contributor Author

ayushnau commented Jun 2, 2024

@smoya updated can you check now.

@smoya
Copy link
Member

smoya commented Jun 4, 2024

There is an issue with our tests in CI when running under macos. It is not the first time we find this.
We need to investigate what the issue is, but not linked to this PR.

@smoya
Copy link
Member

smoya commented Jun 4, 2024

We can't merge this PR until #1001 gets solved. Working on it.

Copy link

sonarqubecloud bot commented Jun 5, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@ayushnau
Copy link
Contributor Author

ayushnau commented Jun 5, 2024

@smoya please check i have updated the pr.

@smoya
Copy link
Member

smoya commented Jun 6, 2024

@jonaslagoni can you give your 👍 now?

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

@smoya if you are happy with it, go for it 👍 Dont see anything from my side

@smoya
Copy link
Member

smoya commented Jun 6, 2024

@smoya if you are happy with it, go for it 👍 Dont see anything from my side

I only have one concern. I want to merge this ASAP and release it (so we can be sure everything works as expected, including CI pipeline etc). The point is that this is a chore and atm I don't see any opened PR with a mergeable status we could merge right after this one in order to do a meaningful release.
Forcing a release for this without providing any new feature or fix is not very desirable...

Any idea or suggestion @jonaslagoni ?

@jonaslagoni
Copy link
Member

I think it's fine regardless, do what is easiest don't think too much about it 🙂

@smoya smoya changed the title chore: introduce the turborepo to the parser.js fix: introduce the turborepo to the parser.js Jun 7, 2024
@smoya smoya changed the title fix: introduce the turborepo to the parser.js feat: introduce the turborepo to the parser.js Jun 7, 2024
@smoya
Copy link
Member

smoya commented Jun 7, 2024

I'm gonna merge this as a new minor version (feat). I prefer to fail fast than cry late.

@smoya
Copy link
Member

smoya commented Jun 7, 2024

/rtm

@smoya smoya removed the do-not-merge label Jun 7, 2024
@asyncapi-bot asyncapi-bot merged commit 0dd2bfc into asyncapi:master Jun 7, 2024
17 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 3.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@smoya
Copy link
Member

smoya commented Jun 7, 2024

I'm gonna merge this as a new minor version (feat). I prefer to fail fast than cry late.

Well, failed fast 😆

[7:50:15 AM] [semantic-release] [@semantic-release/npm] › ℹ Skip publishing to npm registry as package.json's private property is true

See https://github.com/asyncapi/parser-js/actions/runs/9413515248/job/25930521532#step:9:65

@ayushnau I believe you tested this as per your previous comments (to publish to your own NPM account). Anyway, we need to fix this asap as `3.1.0 published in GH is also wrong, since it is a release of the whole repo not the individual app(s).

Btw, I removed such tags and release from GH.

@smoya
Copy link
Member

smoya commented Jun 7, 2024

The issue comes with semantic-release and it's lack of support for monorepo. We should move into Changesets as Studio did: https://github.com/asyncapi/studio/blob/master/doc/adr/0002-use-changesets-for-publishing-and-versioning.md

I'm reverting this PR. @ayushnau Feel free to open a new one with a version that really releases each package individually.

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

7 participants