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

Monorepo setup with task runners, testing lib and API consistency tools #8

Merged
merged 6 commits into from
May 18, 2024

Conversation

lukasIO
Copy link
Contributor

@lukasIO lukasIO commented May 17, 2024

this adds a couple of things:

  • turbo as a monorepo task runner
  • a root package.json for easy script invocation across the monorepo
  • vitest as a testrunner
  • rushstack api-extractor to enable CI checks for API consistency (once we need it, too early right now in pre-release)
  • eslint+prettier combined setup taken from the components-js repo

TODO:

  • add changesets CLI setup (would want to tackle this in a separate PR as this one is already a monster)

@lukasIO lukasIO marked this pull request as ready for review May 17, 2024 15:38
@lukasIO lukasIO requested review from nbsp and davidzhao May 17, 2024 15:38
Copy link
Member

@nbsp nbsp left a comment

Choose a reason for hiding this comment

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

looks okay, a few nits

also if you could add turbo to flake.nix nativeBuildInputs

.github/workflows/build.yml Show resolved Hide resolved
.gitignore 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.

is this needed? it's more verbose but i don't know if it helps readability.
either way the REUSE header should be on top

Copy link
Contributor Author

@lukasIO lukasIO May 17, 2024

Choose a reason for hiding this comment

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

this is just a result of the prettier + eslint config applied and running pnpm format:write. REUSE header is still on top, I think?

Copy link
Member

Choose a reason for hiding this comment

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

it got a bit overzealous and put it on top of REUSE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hah, now I see it!

examples/src/minimal.ts Outdated Show resolved Hide resolved
@nbsp
Copy link
Member

nbsp commented May 17, 2024

also once this is ready please give me time to try and rebase this commit history line into something more parseable

examples/package.json Outdated Show resolved Hide resolved
@lukasIO
Copy link
Contributor Author

lukasIO commented May 17, 2024

also once this is ready please give me time to try and rebase this commit history line into something more parseable

generally we do squash-merge commits across all repos AFAIK, so this shouldn't be an issue?

.eslintrc Outdated Show resolved Hide resolved
@nbsp
Copy link
Member

nbsp commented May 17, 2024

also once this is ready please give me time to try and rebase this commit history line into something more parseable

generally we do squash-merge commits across all repos AFAIK, so this shouldn't be an issue?

this is a Very big commit to squash, it does a couple of things (parity, testing, turbo, API, general linting).

@lukasIO
Copy link
Contributor Author

lukasIO commented May 17, 2024

this is a Very big commit to squash, it does a couple of things (parity, testing, turbo, API, general linting).

yeah, sorry about that. It wasn't planned like that at all. It just suddenly grew out of proportion 😬

},
"peerDependencies": {
"typescript": "^5.0.0"
},
"dependencies": {
"@livekit/agents": "workspace:*",
"@livekit/elevenlabs": "workspace:*",
"@livekit/agents-plugin-elevenlabs": "workspace:*",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just for context: the idea behind renaming this was to have a standard way of naming agent plugins in the future, following an example of rollup plugin conventions.
There officially maintained plugins are named @rollup/plugin-official-plugin and community plugins should be named rollup-plugin-my-plugin

@lukasIO
Copy link
Contributor Author

lukasIO commented May 17, 2024

@nbsp do you want to rebase the commits before merging? (I can also do it, just let me know!)

@nbsp nbsp force-pushed the lukas/conform-to-other-repos branch from 15f7c74 to 81994b1 Compare May 17, 2024 20:30
@nbsp nbsp force-pushed the lukas/conform-to-other-repos branch from 81994b1 to 257b199 Compare May 17, 2024 20:32
Co-authored-by: lukasIO <[email protected]>
@nbsp nbsp force-pushed the lukas/conform-to-other-repos branch from 257b199 to e37a48c Compare May 17, 2024 20:32
@nbsp
Copy link
Member

nbsp commented May 17, 2024

@lukasIO rebased the whole thing, made sure nothing was lost. let me know if these commits make sense and i'll merge

@lukasIO
Copy link
Contributor Author

lukasIO commented May 18, 2024

looks great, thank you 🙏

@nbsp nbsp merged commit e37a48c into main May 18, 2024
1 check passed
@nbsp nbsp deleted the lukas/conform-to-other-repos branch May 18, 2024 08:31
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.

2 participants