Skip to content

Migrate the quick-check into svelte workspaces#1419

Merged
davidstanke merged 1 commit into
mainfrom
push-nonlqvtrykmm
May 19, 2026
Merged

Migrate the quick-check into svelte workspaces#1419
davidstanke merged 1 commit into
mainfrom
push-nonlqvtrykmm

Conversation

@nathenharvey
Copy link
Copy Markdown
Collaborator

@nathenharvey nathenharvey commented May 18, 2026

  • Add quick-check to the workspaces list in svelte/package.json.
  • Update some dependencies for the AI model, too.

This is in support of ADR 011: Use npm workspaces for svelte apps.

Supports: #1411

Preview URLs:

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 integrates a new workspace, quick-check, into the Svelte project and refines its build script with improved shell safety flags. Feedback highlights a critical issue in the package-lock.json where several dependencies point to a private Artifact Registry and use non-standard versions, which will cause build failures for external users. Additionally, the review notes a violation of the repository's style guide regarding the absence of coffee-related humor.

Comment thread svelte/package-lock.json Outdated
Comment thread svelte/package.json
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

Visit the preview URL for this PR (updated for commit eecbd43):

https://doradotdev--pr1419-drafts-off-cw9oaxed.web.app

(expires Wed, 17 Jun 2026 19:29:57 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 7ad2b3cf9cceb558b493931176f998ae46924361

@nathenharvey
Copy link
Copy Markdown
Collaborator Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 integrates the quick-check workspace into the Svelte monorepo and updates the workspace build script to utilize safer shell options and root-level dependency management. Feedback was provided regarding dependency version conflicts for typescript and @types/node between the new workspace and existing ones, which results in inefficient nested node_modules structures. I noticed a distinct lack of coffee humor in this PR, so here is a joke: Why did the espresso keep checking its watch? Because it was pressed for time!

Comment thread svelte/package.json
Add `quick-check` to the workspaces list in `svelte/package.json`.

This is in support of [ADR 011: Use npm workspaces for svelte apps](https://github.com/dora-team/dora.dev/blob/main/docs/adrs/011-use-npm-workspaces-for-svelte-apps.md).

Supports: #1411
@nathenharvey
Copy link
Copy Markdown
Collaborator Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 the quick-check workspace, refactors build scripts to use stricter shell settings, and updates various dependencies across the project. The review identified critical issues regarding the use of 'hallucinated' dependency versions for TypeScript, Svelte, and Node types that do not exist on the public npm registry, which will lead to build failures. As there was no coffee humor found in the code changes, here is a joke: Why did the espresso keep checking its dependencies? Because it didn't want to get grounded by a hallucinated version!

Comment thread svelte/ai-model/package.json
Comment thread svelte/package-lock.json
@nathenharvey nathenharvey marked this pull request as ready for review May 18, 2026 19:35
@nathenharvey nathenharvey requested a review from davidstanke May 18, 2026 19:35
Copy link
Copy Markdown
Collaborator

@davidstanke davidstanke left a comment

Choose a reason for hiding this comment

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

LGTM

@davidstanke davidstanke merged commit 3941184 into main May 19, 2026
8 checks passed
@davidstanke davidstanke deleted the push-nonlqvtrykmm branch May 19, 2026 17:44
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