-
Notifications
You must be signed in to change notification settings - Fork 0
Full turbo #433
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
base: release
Are you sure you want to change the base?
Full turbo #433
Conversation
so they don't collide with other instances
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @pmelab, 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!
Summary of Changes
This pull request aims to fully integrate Turborepo into the project to streamline development workflows. It focuses on caching Drupal installations, file watching, and dev mode for various applications like CMS, Publisher, Storybook, and Schema Tests. The primary goal is to automate tasks and provide an easy way to start all necessary services, automatically re-running prep tasks when code changes.
Highlights
- Turborepo Integration: The PR introduces Turborepo configurations to manage dependencies and caching for various tasks across different applications (CMS, Preview, Publisher, Schema, UI, Webform-export).
- CMS Workflow Improvements: The CMS application now uses Turborepo to manage database preparation, composer dependencies, and configuration imports, aiming for faster and more reliable development cycles.
- Preview App Enhancements: The Preview application gains a 'dev' script in
package.jsonand a corresponding Turborepo configuration to streamline the development process. - Schema Package Updates: The Schema package's build process is refined with specific 'prep' tasks managed by Turborepo, ensuring efficient code generation and type checking.
- UI Package Optimization: The UI package now includes Turborepo configurations for development, build, and testing, with dependencies on webform snapshots for consistent UI testing.
- Webform Export Testing: The webform-export package now uses playwright to generate webform snapshots, and the playwright config is updated to run a php server.
- Test setup: The test setup is updated to start a php server using
get-portandwait-onto ensure the server is running before the tests are executed.
Changelog
Click here to see the changelog
- apps/cms/prep-database.sh
- Removed the conditional check for the SQLite database file. Now
pnpm drupal-installis always executed. - Removed
pnpm export-webforms.
- Removed the conditional check for the SQLite database file. Now
- apps/cms/turbo.json
- Removed
turbo-seed.txtandconfig/**from the inputs for theprep:databasetask. - Removed
../../packages/ui/static/stories/webforms/**from the outputs for theprep:databasetask. - Added a
config:importtask to import configurations, depending onprep:database. - Added a
cleartask depending onconfig:importand^prep. - Added a
devtask depending onclear.
- Removed
- apps/preview/package.json
- Added a
devscript that executesnode build/index.js.
- Added a
- apps/preview/turbo.json
- Added a
devtask depending onprep:appandprep:server.
- Added a
- apps/publisher/turbo.json
- Added a
devtask depending on@custom/cms#config:import.
- Added a
- packages/@amazeelabs/silverback-iframe/turbo.json
- Added
src/**to the inputs for thepreptask. - Updated the outputs for the
preptask todist/**.
- Added
- packages/schema/package.json
- Removed the
prepscript.
- Removed the
- packages/schema/turbo.json
- Updated the
preptask to depend onprep:directives,prep:codegen,prep:scripts, andprep:types.
- Updated the
- packages/ui/package.json
- Removed the
prepscript.
- Removed the
- packages/ui/turbo.json
- Added a dependency on
@custom/webform-export#webform-snapshotsto thebuildtask. - Added a
devtask depending on^prepand@custom/webform-export#webform-snapshots.
- Added a dependency on
- packages/webform-export/package.json
- Added
CI=trueto thewebform-snapshotsscript.
- Added
- packages/webform-export/playwright.config.ts
- Updated the
webServerconfiguration to usephp -Sinstead ofpnpm run --filter "@custom/cms" start. - Added environment variables to the
webServerconfiguration. - Set the reporter to null.
- Updated the
- packages/webform-export/turbo.json
- Added a
webform-snapshotstask depending ontest:staticand@custom/cms#config:import.
- Added a
- pnpm-lock.yaml
- Updated dependencies.
- tests/schema/lib.ts
- Updated the GraphQL endpoint URL to use the
TEST_URIenvironment variable.
- Updated the GraphQL endpoint URL to use the
- tests/schema/package.json
- Removed the
startscript. - Removed
start-server-and-testdependency. - Added
@types/wait-on,get-port, andwait-ondependencies.
- Removed the
- tests/schema/setup.ts
- Added a setup script to start a PHP server using
get-portandwait-on.
- Added a setup script to start a PHP server using
- tests/schema/turbo.json
- Added a
devtask depending on@custom/cms#config:import.
- Added a
- tests/schema/vitest.config.ts
- Added a vitest config file to configure the global setup.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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.
The Turbo's hum, a caching dream,
Speeds builds, it would seem.
From Drupal's core to UI's gleam,
A faster workflow, supreme.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces Turborepo to manage Drupal installation caching, file watching, and development mode. While the goal of streamlining the development process is commendable, some changes might introduce inefficiencies or inconsistencies. The removal of the database check in prep-database.sh and the missing dependency in packages/webform-export/turbo.json are the primary concerns.
Summary of Findings
- Unnecessary database re-installation: The
prep-database.shscript now reinstalls the Drupal database on every run, which is inefficient and potentially time-consuming. The previous logic checked for the existence of the SQLite database file before reinstalling, which is more efficient. - Missing config import dependency: The
webform-snapshotstask inpackages/webform-export/turbo.jsonis missing a dependency on@custom/cms#config:import. This means that the webform snapshots might be generated before the configuration is imported, leading to incorrect snapshots. - Missing cache configuration: The
clearanddevtasks inapps/cms/turbo.jsonandtests/schema/turbo.jsonshould havecache: falseto prevent caching issues during development.
Merge Readiness
The pull request is not ready for merging. The critical issue of unnecessary database re-installation must be addressed to avoid performance degradation. The high severity issue of the missing config import dependency should also be addressed. I am unable to directly approve this pull request, and recommend that these issues be addressed before merging, and that others review and approve this code before merging.
apps/cms/prep-database.sh
Outdated
| pnpm drupal-install | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change removes the check for the existence of the SQLite database file. This means that pnpm drupal-install will be run on every invocation of this script, even if the database already exists. This is inefficient and will slow down development. Consider restoring the previous logic to avoid unnecessary database re-installation.
| pnpm drupal-install | |
| if ! test -f web/sites/default/files/.sqlite; then | |
| pnpm drupal-install | |
| fi |
it needs a root level install and configuration
Description of changes
Attempt to use Turborepo for everything. Drupal installation caching, file watching and dev mode.
Motivation and context
Provide an easy way to start up all necessary services and auto-run necessary commands when the related files are changed.
Simply run
pnpm turbo watch dev --ui tuiin the root folder. That should bring up Drupal, Publisher, Storybook and Schema Tests and automatically re-run necessary prep tasks if code (e.g. schema, operations, fragments, components ...) change.How has this been tested?