-
-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor/merge shared into client #39
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
Conversation
WalkthroughThis change removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Server
participant Client
participant FFI
participant UI_Generator
Server->>Client: Import configtype, contenttypes, version, etc.
Client->>FFI: Call version() via JS FFI
UI_Generator->>Client: Generate themes_generated.gleam in client UI dir
Server->>Client: Use client modules for config and types
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
cynthia_websites_mini_server/src/cynthia_websites_mini_server.gleam (1)
4-6: Dependency inversion warning.The server now imports the client package for types. This creates a circular conceptual dependency (client → server at runtime, server → client at build-time).
Consider extracting the truly shared types (e.g.configtype,contenttypes,timestamps) into a smallmini_commonlibrary to avoid coupling UI code into the server binary.generate-ffi (1)
49-49: Update variable name to reflect the new path.The variable
shared_ui_dirnow points to the client package directory, making the variable name misleading. Consider renaming it toclient_ui_dirfor clarity.-const shared_ui_dir = path.join(config_root, "/cynthia_websites_mini_client/src/cynthia_websites_mini_client/ui"); +const client_ui_dir = path.join(config_root, "/cynthia_websites_mini_client/src/cynthia_websites_mini_client/ui");And update the subsequent references:
-if (!existsSync(shared_ui_dir)) { - mkdirSync(shared_ui_dir, { recursive: true }); +if (!existsSync(client_ui_dir)) { + mkdirSync(client_ui_dir, { recursive: true }); } writeFileSync( - path.join(shared_ui_dir, "themes_generated.gleam"), + path.join(client_ui_dir, "themes_generated.gleam"), themes_gleam_content );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.gitignore(1 hunks)cynthia_websites_mini_client/gleam.toml(1 hunks)cynthia_websites_mini_client/manifest.toml(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client.gleam(2 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/configtype.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/configurable_variables.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/messages.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/model_type.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/pageloader/postlistloader.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/paints.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/view.gleam(1 hunks)cynthia_websites_mini_client/test/cynthia_websites_mini_client_test.gleam(1 hunks)cynthia_websites_mini_server/gleam.toml(1 hunks)cynthia_websites_mini_server/manifest.toml(2 hunks)cynthia_websites_mini_server/src/cynthia_websites_mini_server.gleam(2 hunks)cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam(1 hunks)cynthia_websites_mini_server/src/cynthia_websites_mini_server/mutable_model_type.gleam(1 hunks)cynthia_websites_mini_server/src/cynthia_websites_mini_server/static_routes.gleam(2 hunks)cynthia_websites_mini_server/src/cynthia_websites_mini_server/web.gleam(1 hunks)cynthia_websites_mini_server/test/cynthia_websites_mini_server_test.gleam(1 hunks)cynthia_websites_mini_shared/.gitignore(0 hunks)cynthia_websites_mini_shared/gleam.toml(0 hunks)cynthia_websites_mini_shared/manifest.toml(0 hunks)cynthia_websites_mini_shared/src/cynthia_websites_mini_shared.gleam(0 hunks)generate-ffi(2 hunks)mise.toml(2 hunks)mise/tasks/new-release(2 hunks)
💤 Files with no reviewable changes (4)
- cynthia_websites_mini_shared/.gitignore
- cynthia_websites_mini_shared/src/cynthia_websites_mini_shared.gleam
- cynthia_websites_mini_shared/gleam.toml
- cynthia_websites_mini_shared/manifest.toml
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery.gleam (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_server/src/cynthia_websites_mini_server.gleam (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_server/gleam.toml (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_server/src/cynthia_websites_mini_server/static_routes.gleam (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_client/src/cynthia_websites_mini_client.gleam (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_client/gleam.toml (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_client/manifest.toml (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_server/manifest.toml (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
🔇 Additional comments (27)
cynthia_websites_mini_client/src/cynthia_websites_mini_client/model_type.gleam (1)
1-1: Import path switch looks correct
configtypenow resides in the client package; this new path compiles and aligns with the overall consolidation effort.cynthia_websites_mini_client/src/cynthia_websites_mini_client/messages.gleam (1)
1-1: Consistent path updateSame change here – good to see all client-side modules now pull
configtypefrom the client package.cynthia_websites_mini_server/src/cynthia_websites_mini_server/web.gleam (1)
5-5: No lingeringcynthia_websites_mini_shared/imports detected
Ranrg --line-number 'cynthia_websites_mini_shared/'and confirmed there are no remaining references 🎉 Safe to proceed!cynthia_websites_mini_client/src/cynthia_websites_mini_client/pageloader/postlistloader.gleam (1)
1-1: Post-loader import updated
PostDatanow comes from the client namespace – looks good.cynthia_websites_mini_client/src/cynthia_websites_mini_client/configtype.gleam (1)
1-1: Path realigned; keep an eye on transitive depsThe type alias
Contentis now sourced from the client package, matching the rest of the refactor. Ensuregleam.toml/manifest.tomlhave removed the shared dependency (they appear to be, per PR summary).cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery.gleam (1)
1-2: LGTM: Import paths correctly updated for package consolidation.The import changes align with the refactoring to consolidate shared functionality into the client package.
cynthia_websites_mini_client/src/cynthia_websites_mini_client/configurable_variables.gleam (1)
4-4: Good practice: Keeping documentation in sync with code changes.The comment correctly reflects the new module path after the package consolidation.
cynthia_websites_mini_server/src/cynthia_websites_mini_server/mutable_model_type.gleam (1)
1-1: Confirm server→client dependency is intentionalRe-checks show:
- No remaining references to
cynthia_websites_mini_shared.- Multiple server modules now import from
cynthia_websites_mini_client/configtype(and other client submodules).- No imports from server code appear in the client package (no circular dependency detected).
Please verify that this architectural decision—having the server depend on the client package for shared types/config—is intentional. If these types truly span both server and client, consider extracting them into a separate “shared” package to make the dependency direction clearer.
Key server files importing client code:
- src/cynthia_websites_mini_server/mutable_model_type.gleam
- src/cynthia_websites_mini_server/config.gleam
- src/cynthia_websites_mini_server/web.gleam
- src/cynthia_websites_mini_server/static_routes.gleam
cynthia_websites_mini_client/test/cynthia_websites_mini_client_test.gleam (1)
2-2: LGTM: Test import correctly updated.The import change ensures tests reference the correct module location after the package consolidation.
cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/paints.gleam (1)
1-1: LGTM: Import paths consistently updated.Both
configtypeandui/themes_generatedimports correctly reflect the new package structure after consolidating shared functionality into the client package.Also applies to: 4-4
cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam (1)
3-5: No remainingcynthia_websites_mini_sharedimports found
Yourrg -n "cynthia_websites_mini_shared"returned no matches, indicating the migration looks complete.
However, to be absolutely sure there are no hidden references, please run one more verification across your entire workspace:# Double-check for any lingering shared references git grep -n "cynthia_websites_mini_shared" .cynthia_websites_mini_client/src/cynthia_websites_mini_client/view.gleam (1)
1-1: Import path looks good.Nothing else changed in this file, the rename is straightforward and safe.
cynthia_websites_mini_server/src/cynthia_websites_mini_server.gleam (1)
44-45:version()call might be async – confirm signature.If the new FFI returns a
Promise(String)(common when bridging JS ↔ Gleam), logging it directly will print[object Promise].
Make sureversion()has typeString, orawaitit beforeconsole.log..gitignore (1)
137-138: Updated ignore path acknowledged.The generated theme file is now ignored in its new location – nice catch.
cynthia_websites_mini_server/gleam.toml (1)
32-33: LGTM! Dependency migration implemented correctly.The change from
cynthia_websites_mini_sharedtocynthia_websites_mini_clientaligns with the refactoring objectives and uses the correct local path dependency.mise/tasks/new-release (1)
125-127: LGTM! Release script updated correctly for package migration.The removal of
cynthia_websites_mini_shared/gleam.tomlfrom the release targets is consistent with the migration to consolidate functionality into the client package.cynthia_websites_mini_server/src/cynthia_websites_mini_server/static_routes.gleam (2)
3-5: LGTM! Import migration implemented correctly.The import statements have been properly updated to use the client package instead of the shared package.
33-33: FFI binding implementation verifiedThe
version_ffi.tsfile exists atcynthia_websites_mini_client/src/cynthia_websites_mini_client/version_ffi.ts, confirming the FFI forversion()is in place. No further action required.cynthia_websites_mini_client/gleam.toml (2)
10-10: LGTM! Fixed spacing in version specification.Removed the leading space from the
gleam_stdlibversion specification.
11-14: LGTM! Improved dependency versioning strategy.The change from exact versions to version ranges (e.g.,
">= 0.5.9 and < 1.0.0") is a best practice that provides better compatibility while maintaining stability within major versions.cynthia_websites_mini_client/src/cynthia_websites_mini_client.gleam (2)
3-5: LGTM! Import statements updated correctly.The imports have been properly migrated from the shared package to the client package namespace.
537-538: ✅ FFI Implementation Verified
- The file
cynthia_websites_mini_client/src/cynthia_websites_mini_client/version_ffi.tsexists.- It correctly exports
my_own_version(): string, returning theversionfrompackage.json.No further changes required.
generate-ffi (1)
49-56: LGTM! Path migration is consistent.The path changes correctly redirect theme generation and Tailwind content scanning from the shared package to the client package, aligning with the broader refactoring.
Also applies to: 70-70
mise.toml (1)
75-75: LGTM! Task dependencies correctly updated.The removal of
clean-sharedandfmt-shareddependencies aligns perfectly with the broader refactoring that eliminates the shared package. The remaining dependencies on client and server tasks are appropriate.Also applies to: 156-156
cynthia_websites_mini_client/manifest.toml (1)
6-6: Approve package version updates.The package version updates appear to be a coordinated upgrade across the dependency tree. The version constraint changes from fixed versions to ranges (e.g.,
lustrefrom5.0.2to>= 5.0.2 and < 6.0.0) provide better flexibility for dependency resolution while maintaining backward compatibility.Also applies to: 9-9, 15-15, 21-21, 27-27, 30-30, 32-33, 36-36
cynthia_websites_mini_server/manifest.toml (2)
10-10: LGTM! Dependency migration correctly implemented.The change from
cynthia_websites_mini_sharedtocynthia_websites_mini_clientis correctly implemented in both the packages list and requirements section, with the appropriate local path dependency.Also applies to: 58-58
18-18: Approve dependency additions and version updates.The addition of multiple new dependencies (
gleam_erlang,gleam_httpc,gleam_otp,gleam_time,houdini,jot,lustre,modem,odysseus,rsvp,splitter) correctly reflects the migration of functionality from the removed shared package. The version updates maintain compatibility while providing access to latest features and bug fixes.Also applies to: 21-21, 24-24, 27-27, 32-34, 36-38, 40-40, 43-43, 45-45, 48-48
cynthia_websites_mini_server/test/cynthia_websites_mini_server_test.gleam
Show resolved
Hide resolved
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cynthia_websites_mini_server/gleam.toml (1)
17-22: Re-consider hard-pinning every shared dependency.Locking
gleam_javascript,gleam_community_colour,gleam_json,gleam_http, andgleam_fetchto a single patch version makes the server and client easily drift out of sync and forces dual bumps. A looser semver range (e.g.~> 1.0) or relying on the client’s transitives avoids diamond-dependency clashes and reduces maintenance overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cynthia_websites_mini_client/gleam.toml(1 hunks)cynthia_websites_mini_server/gleam.toml(1 hunks)cynthia_websites_mini_server/manifest.toml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cynthia_websites_mini_client/gleam.toml
- cynthia_websites_mini_server/manifest.toml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
cynthia_websites_mini_server/gleam.toml (1)
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
🔇 Additional comments (2)
cynthia_websites_mini_server/gleam.toml (2)
27-27: Unpinned Git ref undermines reproducible builds.
bungibindiestracksmain, meaning every CI run can fetch a different commit. Pin to an immutable commit hash or tag:-bungibindies = { git = "https://github.com/strawmelonjuice/bungibindies.git", ref = "main" } +bungibindies = { git = "https://github.com/strawmelonjuice/bungibindies.git", ref = "d34db33f" } # ← replace with actual commit
28-28: Localpathdependency prevents publishing to Hex.
cynthia_websites_mini_clientis referenced via a relative path. Hex packaging will fail unless the dependency is also vendored. If public release is a goal, swap to a normal versioned dependency after the refactor stabilises.
Summary by CodeRabbit
Refactor
New Features
Chores