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 monorepo to Generator Repo #1186

Closed
wants to merge 27 commits into from
Closed

feat: introduce monorepo to Generator Repo #1186

wants to merge 27 commits into from

Conversation

ayushnau
Copy link
Contributor

Changes Added.

  1. Turborepo Added.
  2. Converted the generator repo to app, inside the turborepo.
  3. Added the required scripts of the turborepo to the root package.json file.

Related issue(s)

Resolves #1044

@aeworxet
Copy link

@asyncapi/bounty_team

@asyncapi-bot asyncapi-bot added the bounty AsyncAPI Bounty program related label label Apr 16, 2024
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

Some stuff needs update because of docs related links and automation:

  • doc hooks.md need update of links pointing to GH that would break, change them to relative link from website, which will be /docs/tools/generator/api
  • workflow update-docs-in-website needs an update of link that now point to /generator/docs/, and probably should point to /generator/apps/generator/docs/

@derberg
Copy link
Member

derberg commented Apr 16, 2024

additionally:

  • update PR title to match the reality - filters are not here in first iteration
  • you have merge conflicts
  • tests are failing, I suggest running them locally and integration tests fails with Error: An error occurred trying to start process '/usr/bin/bash' with working directory '/home/runner/work/generator/generator/./test/test-project'. No such file or directory which probably means some paths in tests also need an update
  • also regular PR testing workflow fails on installation of dependencies, like npm ci is not working with turbo 🤔

@AyushNautiyalDeveloper
Copy link

Some stuff needs update because of docs related links and automation:

* doc hooks.md need update of links pointing to GH that would break, change them to relative link from website, which will be `/docs/tools/generator/api`

* workflow update-docs-in-website needs an update of link that now point to `/generator/docs/`, and probably should point to `/generator/apps/generator/docs/`
  1. in first point you mean https://github.com/asyncapi/generator/blob/master/docs/api.md to https://github.com/asyncapi/generator/blob/master/apps/generator/docs/api.md

@derberg
Copy link
Member

derberg commented Apr 17, 2024

@AyushNautiyalDeveloper no, it just should be /docs/tools/generator/api as I wrote, a relative link, asyncapi.com relative - add it to asyncapi.com and you will see what I mean -> https://www.asyncapi.com/docs/tools/generator/api

links in docs do not have to work in github in markdown, they need to work in website, where people read them

@ayushnau
Copy link
Contributor Author

@derberg

  1. updated all the links.
  2. The Integration tests are working now.
  3. The npm ci is working fine.

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

  • why we have apps/generator/.gitignore instead of having one gitignore in root?

  • I believe apps/generator/assets/readme-banner.png must stay in the root

  • apps/generator/.releaserc should also stay in root, right? it is a release config file and should be same for all, no?

  • please add some Development guide to readme, that explains what I need to do locally to start the project, run tests

  • please also explain how pushing to npm will work now. This is current release workflow https://github.com/asyncapi/generator/blob/master/.github/workflows/if-nodejs-release.yml where for example you can see we need npm test available in the root, that will trigger all tests of all projects

  • also, the plan was filters are not in the PR but another one, so we should not have all these filters scripts in package.json

  • did you run all tests on local? are you sure we do not need to make any amendments in test/test-project/test.sh ?

package.json Outdated
"access": "public"
"devDependencies": {
"prettier": "^3.2.5",
"turbo": "latest"
Copy link
Member

Choose a reason for hiding this comment

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

should we really point to latest? isn't it risky and error prone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made it to the current latest version.

package.json Outdated
"dev": "turbo dev",
"test": "turbo run test",
"lint": "turbo lint",
"format": "prettier --write \"**/*.{ts,tsx,md}\"",
Copy link
Member

Choose a reason for hiding this comment

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

what we need it for? we do not use ts here anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@ayushnau ayushnau changed the title feat: introduce monorepo with filters package included feat: introduce monorepo to Generator Repo Apr 22, 2024
@ayushnau
Copy link
Contributor Author

ayushnau commented May 1, 2024

@derberg

  1. We need gitignore in the apps/generator for the test file that we dont want to be pushed. also adding those commands in root gitignore will not be applicable to all the apps. And also having multiple gitignore files for the turborepo is common pattern
  2. updated the readmd file to correct place. Also I guess the readmd file for the generator should move to generator itself. so that we can add general
  3. Added the required files to root.

Also i found that the test are not running properly with the current version. the issue is still not fixed
finding ways to run the tests.correctly. and currently i am working on this.

@derberg
Copy link
Member

derberg commented May 13, 2024

@ayushnau how is it going, do you need anything?

Copy link

sonarcloud bot commented May 16, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4 Security Hotspots
9.3% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@ayushnau ayushnau closed this by deleting the head repository May 22, 2024
@ayushnau
Copy link
Contributor Author

This pr is moved to #1213

@asyncapi-bot asyncapi-bot removed the bounty AsyncAPI Bounty program related label label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How about a monorepo 😄
5 participants