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 #1008

Merged
merged 50 commits into from
Jul 4, 2024
Merged

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

merged 50 commits into from
Jul 4, 2024

Conversation

ayushnau
Copy link
Contributor

@ayushnau ayushnau commented Jun 9, 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

@ayushnau
Copy link
Contributor Author

ayushnau commented Jun 9, 2024

image

link
@smoya please check added the changeset integration.

@ayushnau ayushnau changed the title Feat changeset Feat: Added the multiparser to the turborepo. Jun 9, 2024
@ayushnau
Copy link
Contributor Author

@smoya I think the changesets changes are ready to merge I have still used the changelog (didnt make it false in the config) cause there is ongoing bug in the changesets which they have not given the solution yet. And this is basically causing changesets still search for the CHANGELOG.md inspite of making it false in the config. here is the link changesets/action#256
Also I was needed to install the @changesets/cli also cause once I run the chagnesets/cli command directly with npx it is installing the cli version and correctly working but then the ci is not able to find the changelog causing the error.

See https://github.com/asyncapi/parser-js/pull/1008/files#r1648443871

Shouldn’t be @changesets/changelog-git ? You don’t need the cli package with such change and maybe that's the reason you needed to install it?

Alternatively, can't we just then get rid of the GH action and do the commands manually maybe?

@smoya added the @changelog/git and its working now . without installing the cli.

smoya
smoya previously approved these changes Jul 3, 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.

I assume you properly tested the release on your own branch and npm account @ayushnau.

LGTM

@ayushnau
Copy link
Contributor Author

ayushnau commented Jul 3, 2024

image tested and it is deployed to npm registry. Just a thing it creates a pr which i have merged manually. which khudad have told that will get merged automatically with the bot, In my case i did by myself.

@ayushnau
Copy link
Contributor Author

ayushnau commented Jul 3, 2024

also just one thing do i need to change this or it is fine
image
the generate asset line (last line. ) cause in testing it doesnt caused any issue.

@smoya
Copy link
Member

smoya commented Jul 4, 2024

also just one thing do i need to change this or it is fine image the generate asset line (last line. ) cause in testing it doesnt caused any issue.

At a glance, I don't see why the PR would break anything so if you tested and the README's toc is being properly updated + the built app is right, let's skip it.

@smoya
Copy link
Member

smoya commented Jul 4, 2024

/rtm

@smoya
Copy link
Member

smoya commented Jul 4, 2024

For some reason, Sonarcloud status is not being reported. I see it is ok in https://sonarcloud.io/summary/new_code?id=asyncapi_parser-js&pullRequest=1008

Trying to push an empty commit so it triggers a new CI build.

@smoya
Copy link
Member

smoya commented Jul 4, 2024

Trying to push an empty commit so it triggers a new CI build.

No success. BTW, I fixed the .sonarcloud.properties file for you.

@smoya
Copy link
Member

smoya commented Jul 4, 2024

I see it is ok in https://sonarcloud.io/summary/new_code?id=asyncapi_parser-js&pullRequest=1008

No, it's not. That instance ran 18 days ago 😮
Google Chrome_p9ttTOi4@2x

Additionally, is pointing to a commit that I can't find within the commits of this branch ad50cc4

@aeworxet
Copy link
Contributor

aeworxet commented Jul 4, 2024

There was an issue with SonarCloud several weeks ago.
There was a long discussion in Slack
https://asyncapi.slack.com/archives/CQVJXFNQL/p1718711108263639
and at the end of the day an issue was opened in .github
asyncapi/.github#306

It seems not to have a simple immediate solution.

@smoya
Copy link
Member

smoya commented Jul 4, 2024

It seems not to have a simple immediate solution.

Yeah, I even tried to reset the config on Sonarcloud with no success.
I disabled the check in master branch now.

@smoya
Copy link
Member

smoya commented Jul 4, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit b2a0f54 into asyncapi:master Jul 4, 2024
9 checks passed
@smoya
Copy link
Member

smoya commented Jul 4, 2024

I disabled the check in master branch now.

Found like it seems a workaround. See #1033 (comment)

@ayushnau
Copy link
Contributor Author

ayushnau commented Jul 4, 2024

@smoya creating the pr for the multiparser.

@smoya
Copy link
Member

smoya commented Jul 4, 2024

For the record, the release didn't work because the release action wasn't triggered when the automated PR was merged.

The guess is that didn't work because of this https://github.com/asyncapi/parser-js/blob/master/.github/workflows/release-with-changesets.yml#L21-L26, however the PR was created as chore (it is ok).

We would need to remove such checks as in https://github.com/asyncapi/studio/blob/master/.github/workflows/release.yml

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.

6 participants