ENG-724 - Refactor workspace group by apps/package folders #332
Conversation
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
…olidating apps and packages into single entries.
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
Author
|
@CodeRabbit full review |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
discourse-graph.code-workspace (1)
9-12: Fix Next.js watcher exclude: should be “.next”, not “next”.Current pattern won’t match the
.nextdirectory."settings": { "files.watcherExclude": { "**/target": true, "**/dist": true, - "**/next": true, + "**/.next": true, }, },
🧹 Nitpick comments (7)
apps/apps.code-workspace (1)
3-5: Optional: add friendly names for folders in the Explorer.Adds clarity when multiple roots are open.
- { "path": "./obsidian" }, - { "path": "./roam" }, - { "path": "./website" }, + { "path": "./obsidian", "name": "Obsidian" }, + { "path": "./roam", "name": "Roam" }, + { "path": "./website", "name": "Website" },packages/packages..code-workspace (1)
3-8: Optional: label folders for clarity.- { "path": "./database" }, - { "path": "./ui" }, - { "path": "./types" }, - { "path": "./eslint-config" }, - { "path": "./tailwind-config" }, - { "path": "./typescript-config" }, + { "path": "./database", "name": "database" }, + { "path": "./ui", "name": "ui" }, + { "path": "./types", "name": "types" }, + { "path": "./eslint-config", "name": "eslint-config" }, + { "path": "./tailwind-config", "name": "tailwind-config" }, + { "path": "./typescript-config", "name": "typescript-config" },packages/.vscode/settings.json (2)
2-10: Consider not hiding.vscodeand*.code-workspacefor discoverability.Hiding them makes it harder to find/edit workspace config.
"files.exclude": { "**/node_modules": true, "**/dist": true, "**/.turbo": true, "**/.next": true, "**/.vercel": true, - "**/.vscode": true, - "**/*.code-workspace": true + // "**/.vscode": true, + // "**/*.code-workspace": true },
11-17: Optional: also exclude node_modules from the file watcher to reduce FS churn.It doesn’t hide it; it just stops watching it.
"files.watcherExclude": { + "**/node_modules": true, "**/target": true, "**/dist": true, "**/.turbo": true, "**/.next": true, "**/.vercel": true }discourse-graph.code-workspace (1)
8-13: Optional: align watcher excludes with sub-workspaces for consistency.Consider adding
.turboand.vercel(and optionallynode_modules) to reduce watchers at the root too."settings": { "files.watcherExclude": { "**/target": true, "**/dist": true, - "**/next": true, + "**/.next": true, + "**/.turbo": true, + "**/.vercel": true, + // Optionally: + // "**/node_modules": true, }, },apps/.vscode/settings.json (2)
2-11: Consider not hiding.vscodeand*.code-workspacefor discoverability."files.exclude": { "**/node_modules": true, "**/dist": true, "**/.turbo": true, "**/.next": true, "**/.vercel": true, - "**/.vscode": true, - "**/*.code-workspace": true, + // "**/.vscode": true, + // "**/*.code-workspace": true, "**/next-env.d.ts": true },
12-18: Optional: also exclude node_modules from the file watcher to reduce overhead."files.watcherExclude": { + "**/node_modules": true, "**/target": true, "**/dist": true, "**/.turbo": true, "**/.next": true, "**/.vercel": true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/.vscode/settings.json(1 hunks)apps/apps.code-workspace(1 hunks)discourse-graph.code-workspace(1 hunks)packages/.vscode/settings.json(1 hunks)packages/packages..code-workspace(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
📚 Learning: 2025-06-25T18:03:52.669Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Applied to files:
packages/packages..code-workspacediscourse-graph.code-workspacepackages/.vscode/settings.json
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Prettier with the project's configuration
Applied to files:
packages/packages..code-workspacepackages/.vscode/settings.jsonapps/.vscode/settings.json
📚 Learning: 2025-06-25T18:11:58.352Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest tsconfig file, eliminating the need to specify explicit paths like `"./tsconfig.json"`.
Applied to files:
packages/packages..code-workspaceapps/.vscode/settings.json
📚 Learning: 2025-06-25T18:11:58.352Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `parserOptions.project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest `tsconfig.json` file for each source file being linted, eliminating the need to specify explicit paths like `"./tsconfig.json"`. This is especially useful for monorepos and projects with multiple TypeScript configurations.
Applied to files:
packages/packages..code-workspaceapps/.vscode/settings.json
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when working with the Roam Extension API
Applied to files:
apps/apps.code-workspace
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when working with the Roam API
Applied to files:
apps/apps.code-workspace
📚 Learning: 2025-05-22T23:50:23.771Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250513173724_content_concept_key.sql:37-60
Timestamp: 2025-05-22T23:50:23.771Z
Learning: For the discourse-graph project, database schema management focuses on the final state in the supabase/schemas directory, not on the individual migration files. When reviewing database changes, consider only the schema definitions in this directory, not potential duplications or conflicts across migration files.
Applied to files:
discourse-graph.code-workspace
📚 Learning: 2025-06-04T11:54:37.239Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/types.gen.ts:336-371
Timestamp: 2025-06-04T11:54:37.239Z
Learning: The team prefers not to manually edit generated files (like Supabase TypeScript types) even when they contain inaccuracies, as manual changes would be overwritten during regeneration. They treat generated files as read-only artifacts.
Applied to files:
packages/.vscode/settings.json
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to apps/website/**/*.{tsx,jsx,ts,js} : Use Tailwind CSS for styling where possible
Applied to files:
apps/.vscode/settings.json
🔇 Additional comments (1)
discourse-graph.code-workspace (1)
3-5: Confirm intent: root “.” plus grouped “Apps” and “Packages”.This yields three roots; it’s fine if intentional (root keeps build files visible), but it can feel duplicative. Just confirming this aligns with team preference.
Contributor
Author
|
@CodeRabbit full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
discourse-graph.code-workspace (1)
8-12: Fix watcher pattern for Next.js and align excludesThe pattern should be "**/.next" (with a dot). Also consider adding ".turbo" and ".vercel" here to match folder-level settings.
Apply:
"files.watcherExclude": { "**/target": true, "**/dist": true, - "**/next": true, + "**/.next": true, + "**/.turbo": true, + "**/.vercel": true, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/.vscode/settings.json(1 hunks)discourse-graph.code-workspace(1 hunks)packages/.vscode/settings.json(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: mdroidian
PR: DiscourseGraphs/discourse-graph#332
File: apps/.vscode/settings.json:1-19
Timestamp: 2025-08-10T03:37:20.973Z
Learning: In VS Code multi-root workspaces with grouped folders (defined using `"name"` property in the workspace file), `.vscode/settings.json` files within the grouped folders are properly loaded and applied when the root workspace is opened. This differs from standard multi-root workspace behavior where subfolder settings might not apply. VS Code's GUI will create these `.vscode/settings.json` files in grouped folders when settings are configured through the interface.
📚 Learning: 2025-06-25T18:03:52.669Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Applied to files:
discourse-graph.code-workspacepackages/.vscode/settings.jsonapps/.vscode/settings.json
📚 Learning: 2025-08-10T03:37:20.973Z
Learnt from: mdroidian
PR: DiscourseGraphs/discourse-graph#332
File: apps/.vscode/settings.json:1-19
Timestamp: 2025-08-10T03:37:20.973Z
Learning: In VS Code multi-root workspaces with grouped folders (defined using `"name"` property in the workspace file), `.vscode/settings.json` files within the grouped folders are properly loaded and applied when the root workspace is opened. This differs from standard multi-root workspace behavior where subfolder settings might not apply. VS Code's GUI will create these `.vscode/settings.json` files in grouped folders when settings are configured through the interface.
Applied to files:
discourse-graph.code-workspacepackages/.vscode/settings.jsonapps/.vscode/settings.json
📚 Learning: 2025-05-22T23:50:23.771Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250513173724_content_concept_key.sql:37-60
Timestamp: 2025-05-22T23:50:23.771Z
Learning: For the discourse-graph project, database schema management focuses on the final state in the supabase/schemas directory, not on the individual migration files. When reviewing database changes, consider only the schema definitions in this directory, not potential duplications or conflicts across migration files.
Applied to files:
discourse-graph.code-workspace
📚 Learning: 2025-06-04T11:54:37.239Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/types.gen.ts:336-371
Timestamp: 2025-06-04T11:54:37.239Z
Learning: The team prefers not to manually edit generated files (like Supabase TypeScript types) even when they contain inaccuracies, as manual changes would be overwritten during regeneration. They treat generated files as read-only artifacts.
Applied to files:
packages/.vscode/settings.json
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when working with the Roam Extension API
Applied to files:
packages/.vscode/settings.jsonapps/.vscode/settings.json
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Prettier with the project's configuration
Applied to files:
packages/.vscode/settings.jsonapps/.vscode/settings.json
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when working with the Roam API
Applied to files:
apps/.vscode/settings.json
📚 Learning: 2025-06-25T18:11:58.352Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `parserOptions.project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest `tsconfig.json` file for each source file being linted, eliminating the need to specify explicit paths like `"./tsconfig.json"`. This is especially useful for monorepos and projects with multiple TypeScript configurations.
Applied to files:
apps/.vscode/settings.json
📚 Learning: 2025-06-25T18:11:58.352Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest tsconfig file, eliminating the need to specify explicit paths like `"./tsconfig.json"`.
Applied to files:
apps/.vscode/settings.json
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to apps/website/**/*.{tsx,jsx,ts,js} : Use Tailwind CSS for styling where possible
Applied to files:
apps/.vscode/settings.json
🔇 Additional comments (3)
discourse-graph.code-workspace (1)
3-5: Workspace grouping LGTMConsolidating to grouped "Apps" and "Packages" folders is cleaner and easier to maintain.
packages/.vscode/settings.json (1)
11-17: Watcher set looks goodRules align with expected build artifacts for packages. No issues spotted.
apps/.vscode/settings.json (1)
12-18: Watcher set looks good and consistentMatches expected app build dirs and complements the workspace-level settings.
maparent
approved these changes
Aug 11, 2025
Collaborator
maparent
left a comment
maparent
left a comment
There was a problem hiding this comment.
+10 for Removing build files.
0 for regrouping. I would not do it but I see the virtues of doing so. It's less confusing to follow the directory structure, but it means more clicks to reach any components. No right or wrong decision there, it's a trade-off.
Contributor
Author
100%. Let's bring it up in the dev meeting and have a vote. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
*This PR introduces some more opinionated settings and I'm not married to any decisions. @maparent let me know your thoughts and which you'd like to keep/remove.
This PR continues with the
.code-workspacesstarted in #246 byappsandpackagesdiscourse-graphs) for when they need to be accessedSummary by CodeRabbit