asyncapi / parser-js

AsyncAPI parser for Javascript (browser-compatible too).
Apache License 2.0
118 stars 93 forks source link

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

Closed ayushnau closed 3 months ago

ayushnau commented 3 months ago

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: https://github.com/asyncapi/parser-js/issues/963

ayushnau commented 3 months ago
image

link @smoya please check added the changeset integration.

aeworxet commented 3 months ago

@asyncapi/bounty_team

smoya commented 3 months ago

Just for the record, a conversation happened in the following Slack thread where @KhudaDad414 gave the next steps @ayushnau needs to follow in order to make this PR functional and mergeable.

@ayushnau would you mind summarizing what are you working on atm? Thanks!

ayushnau commented 3 months ago

Just for the record, a conversation happened in the following Slack thread where @KhudaDad414 gave the next steps @ayushnau needs to follow in order to make this PR functional and mergeable.

@ayushnau would you mind summarizing what are you working on atm? Thanks!

@smoya I'm currently working on creating the correct GitHub Actions workflow for releasing this via changeset.

sonarcloud[bot] commented 3 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

ayushnau commented 3 months ago

@smoya please check added the new release file.

ayushnau commented 3 months ago

@smoya

  1. When running changeset without installing @changeset/cli, the npx changeset command are not running so i think we need to install that. I think this is the reason it is installed in the dependencies in the studio repo also.
  2. Also as derberg Suggested to use npx but while using that npx is not able to find a dependencies called @changeset/cli/changelog causing it to fail. So I am saying to use the npm packages instead.
image
ayushnau commented 3 months ago

@smoya i have made the required changes can you check.

ayushnau commented 3 months ago

@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 https://github.com/changesets/action/issues/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.

smoya commented 3 months ago

@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?

ayushnau commented 3 months ago

@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.

ayushnau commented 3 months ago
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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

/rtm

smoya commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 https://github.com/asyncapi/parser-js/commit/ad50cc4a4f15e437c5513a4e06978c8c140695e7

aeworxet commented 3 months ago

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 https://github.com/asyncapi/.github/issues/306

It seems not to have a simple immediate solution.

smoya commented 3 months ago

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 commented 3 months ago

/rtm

smoya commented 3 months ago

I disabled the check in master branch now.

Found like it seems a workaround. See https://github.com/asyncapi/parser-js/issues/1033#issuecomment-2208958397

ayushnau commented 3 months ago

@smoya creating the pr for the multiparser.

smoya commented 3 months ago

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