Open
Conversation
|
|
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
@tolgahan-arikan is attempting to deploy a commit to the Foundry development Team on Vercel. A member of the Team first needs to authorize it. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents a substantial overhaul of the project's foundational structure and development workflow. The primary goal was to modernize the monorepo setup by adopting Turbo, which necessitated updating numerous configuration files and removing outdated tooling. Concurrently, a significant portion of the existing Highlights
Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
This comment was marked as outdated.
This comment was marked as outdated.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and wide-ranging refactoring of the project structure and tooling, moving to a modern monorepo setup with Turborepo and PNPM. It also appears to be part of a major version upgrade, removing a large number of v2 packages and adding boilerplate for new docs and web applications. The changes are generally positive, improving the development setup and modernizing the stack.
My review focuses on the new and modified configuration and application files. I've identified a bug in a hardcoded URL in the new web app and pointed out some dependency inconsistencies that should be addressed for better maintainability.
Given the scale of these changes, a more descriptive pull request title and a detailed body explaining the motivation and the new architecture would be highly beneficial for reviewers and future developers.
Comment on lines
+42
to
+50
| <a | ||
| className={styles.primary} | ||
| href="https://vercel.com/new/clone?demo-description=Learn+to+implement+a+monorepo+with+a+two+Next.js+sites+that+has+installed+three+local+packages.&demo-image=%2F%2Fimages.ctfassets.net%2Fe5382hct74si%2F4K8ZISWAzJ8X1504ca0zmC%2F0b21a1c6246add355e55816278ef54bc%2FBasic.png&demo-title=Monorepo+with+Turborepo&demo-url=https%3A%2F%2Fexamples-basic-web.vercel.sh%2F&from=templates&project-name=Monorepo+with+Turborepo&repository-name=monorepo-turborepo&repository-url=https%3A%2F%2Fgithub.com%2Fvercel%2Fturborepo%2Ftree%2Fmain%2Fexamples%2Fbasic&root-directory=apps%2Fdocs&skippable-integrations=1&teamSlug=vercel&utm_source=create-turbo" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| <Image className={styles.logo} src="/vercel.svg" alt="Vercel logomark" width={20} height={20} /> | ||
| Deploy now | ||
| </a> |
There was a problem hiding this comment.
This "Deploy now" link seems to be copied from the docs app and incorrectly points to root-directory=apps%2Fdocs. It should point to apps/web for this application. Using URLSearchParams to construct the URL makes it more readable and helps prevent such copy-paste errors.
<a
className={styles.primary}
href={`https://vercel.com/new/clone?${new URLSearchParams({
'demo-description': 'Learn to implement a monorepo with a two Next.js sites that has installed three local packages.',
'demo-image': '//images.ctfassets.net/e5382hct74si/4K8ZISWAzJ8X1504ca0zmC/0b21a1c6246add355e55816278ef54bc/Basic.png',
'demo-title': 'Monorepo with Turborepo',
'demo-url': 'https://examples-basic-web.vercel.sh/',
from: 'templates',
'project-name': 'Monorepo with Turborepo',
'repository-name': 'monorepo-turborepo',
'repository-url': 'https://github.com/vercel/turborepo/tree/main/examples/basic',
'root-directory': 'apps/web',
'skippable-integrations': '1',
teamSlug: 'vercel',
utm_source: 'create-turbo'
})}`}
target="_blank"
rel="noopener noreferrer"
>
<Image className={styles.logo} src="/vercel.svg" alt="Vercel logomark" width={20} height={20} />
Deploy now
</a>
Comment on lines
+42
to
+50
| <a | ||
| className={styles.primary} | ||
| href="https://vercel.com/new/clone?demo-description=Learn+to+implement+a+monorepo+with+a+two+Next.js+sites+that+has+installed+three+local+packages.&demo-image=%2F%2Fimages.ctfassets.net%2Fe5382hct74si%2F4K8ZISWAzJ8X1504ca0zmC%2F0b21a1c6246add355e55816278ef54bc%2FBasic.png&demo-title=Monorepo+with+Turborepo&demo-url=https%3A%2F%2Fexamples-basic-web.vercel.sh%2F&from=templates&project-name=Monorepo+with+Turborepo&repository-name=monorepo-turborepo&repository-url=https%3A%2F%2Fgithub.com%2Fvercel%2Fturborepo%2Ftree%2Fmain%2Fexamples%2Fbasic&root-directory=apps%2Fdocs&skippable-integrations=1&teamSlug=vercel&utm_source=create-turbo" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| <Image className={styles.logo} src="/vercel.svg" alt="Vercel logomark" width={20} height={20} /> | ||
| Deploy now | ||
| </a> |
There was a problem hiding this comment.
This hardcoded URL is very long, making it difficult to read and maintain. Consider building it programmatically with URLSearchParams for better readability and easier updates.
<a
className={styles.primary}
href={`https://vercel.com/new/clone?${new URLSearchParams({
'demo-description': 'Learn to implement a monorepo with a two Next.js sites that has installed three local packages.',
'demo-image': '//images.ctfassets.net/e5382hct74si/4K8ZISWAzJ8X1504ca0zmC/0b21a1c6246add355e55816278ef54bc/Basic.png',
'demo-title': 'Monorepo with Turborepo',
'demo-url': 'https://examples-basic-web.vercel.sh/',
from: 'templates',
'project-name': 'Monorepo with Turborepo',
'repository-name': 'monorepo-turborepo',
'repository-url': 'https://github.com/vercel/turborepo/tree/main/examples/basic',
'root-directory': 'apps/docs',
'skippable-integrations': '1',
teamSlug: 'vercel',
utm_source: 'create-turbo'
})}`}
target="_blank"
rel="noopener noreferrer"
>
<Image className={styles.logo} src="/vercel.svg" alt="Vercel logomark" width={20} height={20} />
Deploy now
</a>
| "@types/node": "^20.17.57", | ||
| "@types/react": "18.3.1", | ||
| "@types/react-dom": "18.3.0", | ||
| "typescript": "5.5.4" |
There was a problem hiding this comment.
The TypeScript version here (5.5.4) is different from the one in the root package.json (5.8.3). To ensure consistency and avoid potential type-related issues across the monorepo, it's recommended to use the same TypeScript version everywhere.
Suggested change
| "typescript": "5.5.4" | |
| "typescript": "5.8.3" |
| "@types/node": "^20.17.57", | ||
| "@types/react": "18.3.1", | ||
| "@types/react-dom": "18.3.0", | ||
| "typescript": "5.5.4" |
There was a problem hiding this comment.
The TypeScript version here (5.5.4) is different from the one in the root package.json (5.8.3). To ensure consistency and avoid potential type-related issues across the monorepo, it's recommended to use the same TypeScript version everywhere.
Suggested change
| "typescript": "5.5.4" | |
| "typescript": "5.8.3" |
Dargon789
force-pushed
the
WIP-waas-playfab
branch
from
855ebd0 to
ac996d4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.