Refactor library improvements #18
Conversation
This commit introduces a major architectural refactoring to simplify the library's structure and improve its maintainability. Key changes include: - **Simplified Project Structure:** All top-level facade packages (config, crypt, display, etc.) and the root `core.go` have been removed. All library code now resides directly under the `pkg/` directory. - **Unified Runtime:** A new `pkg/runtime` module with a `New()` constructor has been introduced. This function initializes and wires together all core services, providing a single, convenient entry point for applications. - **Updated Entry Points:** The `cmd/core-gui` application and all examples have been updated to use the new `runtime.New()` initialization. - **Internal Packages:** The `config` and `crypt` packages have been refactored to use an `internal` subdirectory for their implementation. This hides private details and exposes a clean, stable public API. - **Standardized Error Handling:** A new error handling package has been added at `pkg/e`. The `workspace` and `crypt` services have been updated to use this new standard. - **Improved Feature Flagging:** A `IsFeatureEnabled` method was added to the `config` service for more robust and centralized feature flag checks. - **CI and Dependencies:** - A GitHub Actions workflow has been added for continuous integration. - All Go dependencies have been updated to their latest versions. - **Documentation:** All documentation has been updated to reflect the new, simplified architecture, and obsolete files have been removed.
This commit introduces a major architectural refactoring to simplify the library's structure and improve its maintainability. Key changes include: - **Simplified Project Structure:** All top-level facade packages (config, crypt, display, etc.) and the root `core.go` have been removed. All library code now resides directly under the `pkg/` directory. - **Unified Runtime:** A new `pkg/runtime` module with a `New()` constructor has been introduced. This function initializes and wires together all core services, providing a single, convenient entry point for applications. The runtime now accepts the Wails application instance, ensuring proper integration with the GUI. - **Updated Entry Points:** The `cmd/core-gui` application and all examples have been updated to use the new `runtime.New()` constructor and correctly register the runtime as a Wails service. - **Internal Packages:** The `config` and `crypt` packages have been refactored to use an `internal` subdirectory for their implementation. This hides private details and exposes a clean, stable public API. - **Standardized Error Handling:** A new error handling package has been added at `pkg/e`. The `workspace` and `crypt` services have been updated to use this new standard. - **Improved Feature Flagging:** A `IsFeatureEnabled` method was added to the `config` service for more robust and centralized feature flag checks. - **CI and Dependencies:** - A GitHub Actions workflow has been added for continuous integration. - All Go dependencies have been updated to their latest versions. - **Documentation:** All documentation has been updated to reflect the new, simplified architecture, and obsolete files have been removed.
|
Warning Rate limit exceeded@Snider has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (21)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughRefactors the Core framework's architecture from facade-based public APIs to internal-only implementations. Removes public re-export wrappers across service packages, consolidates service registration through a new runtime-based pattern, deletes user-facing documentation, introduces error wrapping utilities, and updates dependencies. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Runtime as runtime.New()
participant Core as core.Core
participant Services as Services
Note over App,Services: Old Flow (Removed)
App->>Core: core.New()
Core->>Core: WithService(config.Register)
Core->>Core: WithService(display.Register)
Core->>Core: WithService(crypt.Register)
Core->>Services: RegisterService(coreService)
Note over App,Services: New Flow (Implemented)
App->>Runtime: runtime.New(app)
Runtime->>Core: core.New()
Runtime->>Core: core.WithName("config", internal.Register)
Runtime->>Core: core.WithName("display", internal.Register)
Runtime->>Core: core.WithName("crypt", internal.Register)
Runtime->>Services: RegisterService(runtimeService)
Runtime-->>App: returns Runtime
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
cmd/core/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sum
📒 Files selected for processing (52)
.github/workflows/ci.yml(1 hunks)cmd/core-gui/main.go(1 hunks)cmd/core-website/docs/core/config.md(0 hunks)cmd/core-website/docs/core/crypt.md(0 hunks)cmd/core-website/docs/core/display.md(0 hunks)cmd/core-website/docs/core/help.md(0 hunks)cmd/core-website/docs/core/io.md(0 hunks)cmd/core-website/docs/core/workspace.md(0 hunks)cmd/core/go.mod(1 hunks)cmd/examples/core-task-change/main.go(1 hunks)config/config.go(0 hunks)config/config_test.go(0 hunks)core.go(0 hunks)core_test.go(0 hunks)crypt/crypt.go(0 hunks)crypt/crypt_test.go(0 hunks)display/display.go(0 hunks)display/display_test.go(0 hunks)docs/core/config.md(0 hunks)docs/core/crypt.md(0 hunks)docs/core/display.md(0 hunks)docs/core/help.md(0 hunks)docs/core/index.md(0 hunks)docs/core/io.md(0 hunks)docs/core/workspace.md(0 hunks)docs/index.md(1 hunks)go.mod(1 hunks)help/help.go(0 hunks)help/help_test.go(0 hunks)i18n/i18n.go(0 hunks)i18n/i18n_test.go(0 hunks)pkg/config/config.go(1 hunks)pkg/config/config_test.go(0 hunks)pkg/config/internal/config_test.go(1 hunks)pkg/config/internal/service.go(1 hunks)pkg/core/core.go(1 hunks)pkg/crypt/crypt.go(1 hunks)pkg/crypt/internal/crypt_test.go(1 hunks)pkg/crypt/internal/service.go(1 hunks)pkg/e/e.go(1 hunks)pkg/help/src/core/config.md(0 hunks)pkg/help/src/core/crypt.md(0 hunks)pkg/help/src/core/display.md(0 hunks)pkg/help/src/core/help.md(0 hunks)pkg/help/src/core/index.md(0 hunks)pkg/help/src/core/io.md(0 hunks)pkg/help/src/core/workspace.md(0 hunks)pkg/runtime/runtime.go(6 hunks)pkg/runtime/runtime_test.go(2 hunks)pkg/workspace/workspace.go(9 hunks)workspace/workspace.go(0 hunks)workspace/workspace_test.go(0 hunks)
💤 Files with no reviewable changes (35)
- cmd/core-website/docs/core/display.md
- docs/core/io.md
- pkg/help/src/core/config.md
- cmd/core-website/docs/core/help.md
- pkg/config/config_test.go
- pkg/help/src/core/crypt.md
- display/display_test.go
- workspace/workspace.go
- cmd/core-website/docs/core/config.md
- config/config.go
- docs/core/index.md
- display/display.go
- cmd/core-website/docs/core/crypt.md
- pkg/help/src/core/workspace.md
- cmd/core-website/docs/core/workspace.md
- docs/core/crypt.md
- i18n/i18n.go
- pkg/help/src/core/io.md
- docs/core/workspace.md
- crypt/crypt_test.go
- pkg/help/src/core/display.md
- help/help_test.go
- docs/core/config.md
- docs/core/help.md
- i18n/i18n_test.go
- core_test.go
- pkg/help/src/core/index.md
- docs/core/display.md
- pkg/help/src/core/help.md
- config/config_test.go
- workspace/workspace_test.go
- cmd/core-website/docs/core/io.md
- crypt/crypt.go
- help/help.go
- core.go
🧰 Additional context used
🧬 Code graph analysis (12)
pkg/core/core.go (2)
pkg/core/interfaces.go (1)
Option(19-19)pkg/core/core_test.go (2)
TestWithService(33-81)TestRegisterService(140-165)
pkg/config/config.go (1)
pkg/config/internal/service.go (4)
Options(20-20)Service(24-38)New(100-102)Register(106-117)
pkg/config/internal/service.go (5)
pkg/runtime/runtime.go (2)
Runtime(19-28)New(100-109)pkg/config/config.go (4)
Options(9-9)Service(12-12)New(15-17)Register(20-22)pkg/crypt/internal/service.go (4)
Options(22-22)Service(25-27)New(47-49)Register(53-60)pkg/workspace/workspace.go (4)
Options(23-23)Service(32-37)New(51-63)Register(68-75)pkg/core/runtime.go (1)
NewRuntime(10-15)
pkg/config/internal/config_test.go (3)
pkg/config/config.go (1)
New(15-17)pkg/config/internal/service.go (1)
New(100-102)pkg/config/config_test.go (2)
TestConfigService(49-132)setupTestEnv(12-35)
cmd/core-gui/main.go (4)
pkg/runtime/runtime.go (1)
New(100-109)pkg/core/core.go (1)
New(15-34)cmd/examples/core-static-di/main.go (1)
main(14-41)pkg/core/runtime.go (2)
core(4-7)r(23-25)
cmd/examples/core-task-change/main.go (2)
pkg/runtime/runtime.go (1)
New(100-109)cmd/examples/core-static-di/main.go (1)
main(14-41)
pkg/runtime/runtime.go (3)
pkg/core/core.go (4)
App(186-191)WithWails(82-87)WithName(72-80)New(15-34)pkg/core/interfaces.go (1)
Option(19-19)cmd/examples/core-static-di/main.go (1)
main(14-41)
pkg/crypt/internal/crypt_test.go (2)
core_test.go (2)
TestPublicAPICompleteness(14-70)compareExports(74-102)crypt/crypt_test.go (1)
TestInterfaceCompliance(28-31)
pkg/workspace/workspace.go (3)
pkg/e/e.go (1)
E(41-43)pkg/config/internal/service.go (1)
Service(24-38)pkg/core/interfaces.go (1)
CreateWorkspace(82-87)
pkg/runtime/runtime_test.go (1)
pkg/runtime/runtime.go (1)
New(100-109)
pkg/crypt/internal/service.go (4)
pkg/crypt/crypt.go (9)
Options(9-9)HashType(15-15)Service(12-12)New(26-28)LTHN(18-18)SHA512(19-19)SHA1(21-21)MD5(22-22)SHA256(20-20)pkg/core/runtime.go (1)
NewRuntime(10-15)pkg/crypt/openpgp/encrypt.go (2)
EncryptPGP(113-164)DecryptPGP(167-233)pkg/core/interfaces.go (1)
EncryptPGP(68-71)
pkg/crypt/crypt.go (2)
pkg/crypt/internal/service.go (10)
Options(22-22)Service(25-27)HashType(30-30)LTHN(33-33)SHA512(34-34)SHA256(35-35)SHA1(36-36)MD5(37-37)New(47-49)Register(53-60)pkg/runtime/runtime.go (1)
New(100-109)
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
13-13: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
16-16: the runner of "actions/setup-go@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 LanguageTool
docs/index.md
[uncategorized] ~11-~11: The official name of this software platform is spelled with a capital “H”.
Context: .../core.help](https://core.help) - Repo: [github.com:Snider/Core](https://github.com/Sni...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
docs/index.md
35-35: Hard tabs
Column: 1
(MD010, no-hard-tabs)
37-37: Hard tabs
Column: 1
(MD010, no-hard-tabs)
38-38: Hard tabs
Column: 1
(MD010, no-hard-tabs)
42-42: Hard tabs
Column: 1
(MD010, no-hard-tabs)
43-43: Hard tabs
Column: 1
(MD010, no-hard-tabs)
44-44: Hard tabs
Column: 1
(MD010, no-hard-tabs)
45-45: Hard tabs
Column: 1
(MD010, no-hard-tabs)
47-47: Hard tabs
Column: 1
(MD010, no-hard-tabs)
48-48: Hard tabs
Column: 1
(MD010, no-hard-tabs)
49-49: Hard tabs
Column: 1
(MD010, no-hard-tabs)
50-50: Hard tabs
Column: 1
(MD010, no-hard-tabs)
51-51: Hard tabs
Column: 1
(MD010, no-hard-tabs)
53-53: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: coverage
- GitHub Check: CodeQL
🔇 Additional comments (12)
pkg/e/e.go (2)
46-51: LGTM! Clean error formatting implementation.The error formatting correctly handles both wrapped and terminal errors, providing clear operational context in the error message.
54-56: LGTM! Proper unwrap implementation.The
Unwrap()method correctly provides compatibility with Go'serrors.Isanderrors.Asfunctions.pkg/workspace/workspace.go (3)
54-54: LGTM! Consistent error wrapping adoption.The adoption of
e.E()for error wrapping provides clear operational context throughout the workspace service.
222-226: Simplify redundant error handling.Similar to the previous case, but we need to preserve the logic. Actually, this is fine as-is.
209-213: Simplify redundant error handling.The explicit error check and wrapping can be simplified to a single line since there's no additional logic between the call and return.
Apply this diff to simplify:
- content, err := s.medium.FileGet(path) - if err != nil { - return "", e.E("workspace.WorkspaceFileGet", "failed to get file", err) - } - return content, nil + content, err := s.medium.FileGet(path) + return content, e.E("workspace.WorkspaceFileGet", "failed to get file", err)Wait, this would always return an error. Let me reconsider:
content, err := s.medium.FileGet(path) if err != nil { return "", e.E("workspace.WorkspaceFileGet", "failed to get file", err) } return content, nilActually, the current implementation is correct as-is because we only want to wrap when there's an error. Discard this comment.
docs/index.md (1)
27-55: LGTM! Documentation aligns with new runtime-based initialization.The Quick Start example correctly demonstrates the new runtime-based initialisation pattern using
runtime.New()and Wails integration.pkg/crypt/crypt.go (2)
9-23: LGTM! Clean delegation to internal implementation.The type aliases correctly maintain the public API surface whilst delegating to internal implementations. This pattern provides a clean separation between public and internal code.
26-33: LGTM! Constructor delegation is well-structured.Both
New()andRegister()correctly delegate to their internal counterparts, maintaining a consistent public API.pkg/crypt/internal/crypt_test.go (1)
1-1: LGTM! Package name aligns with internal structure.The package name change from
crypttointernalcorrectly reflects the file's location within the internal directory structure.pkg/core/core.go (1)
72-79: LGTM! Clean implementation with proper error handling.The function correctly creates a service instance, wraps errors with context including the service name, and delegates registration to
RegisterService, which handles validation (empty name check, duplicate checks, etc.).cmd/core/go.mod (1)
9-10: Verification complete—no issues identified.The
debmepackage is an embed.FS wrapper providing additional functionality, and gosod is an application scaffold library. Both packages are publicly available and maintained by Lea Anthony, creator of Wails. The security advisory check found no known vulnerabilities for either package. The packages are designed to work well together with go:embed, indicating these are intentional, compatible dependencies for the project's architecture.go.mod (1)
12-12: Based on my search, Wails v3.0.0-alpha.10 was released on 6 July 2025, and I cannot find evidence that v3.0.0-alpha.37 exists. The version mentioned in your review comment does not appear in official Wails release channels, changelogs, or GitHub releases pages.
Version v3.0.0-alpha.37 does not appear to exist in Wails releases.
The highest confirmed v3 alpha release is alpha.10 (July 2025). There is no publicly available v3.0.0-alpha.37 release. Verify the actual dependency version in go.mod before proceeding.
Likely an incorrect or invalid review comment.
* feat: Implement TDD contract testing for public API This commit introduces a Test-Driven Development (TDD) workflow to enforce the public API contract. A new `tdd/` directory has been added to house these tests, which are intended to be the starting point for any new features or bug fixes that affect the public interface. The "Good, Bad, Ugly" testing methodology has been adopted for these tests: - `_Good` tests verify the "happy path" with valid inputs. - `_Bad` tests verify predictable errors with invalid inputs. - `_Ugly` tests verify edge cases and unexpected inputs to prevent panics. TDD contract tests have been implemented for the `core` and `config` packages, and the `core.New` function has been hardened to prevent panics from `nil` options. The `README.md` has been updated to document this new workflow. * feat: Add TDD contract tests for all services This commit expands the TDD contract testing framework to cover all services in the application. "Good, Bad, Ugly" tests have been added for the `help`, `i18n`, and `workspace` services. To facilitate testing, the following refactors were made: - `help`: Added a `SetDisplay` method to allow for mock injection. Hardened `Show` and `ShowAt` to prevent panics. - `i18n`: Added a `SetBundle` method to allow for loading test-specific localization files. - `workspace`: Made the `Config` field public and added a `SetMedium` method to allow for mock injection. The TDD tests for the `crypt` service have been skipped due to issues with PGP key generation in the test environment. * CLI code-docgen function (#16) * Refactor CLI structure: move commands to 'dev' package, add docstring generation command, and update Taskfile for new tasks Signed-off-by: Snider <snider@lt.hn> * Add CodeRabbit PR review badge to README Signed-off-by: Snider <snider@lt.hn> --------- Signed-off-by: Snider <snider@lt.hn> --------- Signed-off-by: Snider <snider@lt.hn> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit introduces a major architectural refactoring of the Core library and adds a new, automated documentation system. **Architectural Changes:** * **Unified Runtime:** A new `pkg/runtime` module provides a single `runtime.New()` constructor that initializes and manages all core services. This simplifies application startup and improves maintainability. * **Wails Integration:** The `Runtime` is now correctly integrated with the Wails application lifecycle, accepting the `*application.App` instance and being registered as a Wails service. * **Simplified Project Structure:** All top-level facade packages have been removed, and library code is now consolidated under the `pkg/` directory. * **Internal Packages:** The `config` and `crypt` services now use an `internal` package to enforce a clean separation between public API and implementation details. * **Standardized Error Handling:** The `pkg/e` package has been introduced and integrated into the `workspace` and `crypt` services for consistent error handling. * **Graceful Shutdown:** The shutdown process has been fixed to ensure shutdown signals are correctly propagated to all services. **Documentation:** * **Automated Doc Generation:** A new `docgen` command has been added to `cmd/core` to automatically generate Markdown documentation from the service source code. * **MkDocs Site:** A new MkDocs Material documentation site has been configured in the `/docs` directory. * **Deployment Workflow:** A new GitHub Actions workflow (`.github/workflows/docs.yml`) automatically builds and deploys the documentation site to GitHub Pages. **Quality Improvements:** * **Hermetic Tests:** The config service tests have been updated to be fully hermetic, running in a temporary environment to avoid side effects. * **Panic Fix:** A panic in the config service's `Set` method has been fixed, and "Good, Bad, Ugly" tests have been added to verify the fix. * **CI/CD:** The CI workflow has been updated to use the latest GitHub Actions. * **Code Quality:** Numerous smaller fixes and improvements have been made based on CI feedback.
No description provided.