refactor: optimize facades install/uninstall logic#1177
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1177 +/- ##
==========================================
+ Coverage 68.73% 68.86% +0.12%
==========================================
Files 231 230 -1
Lines 14957 14879 -78
==========================================
- Hits 10281 10246 -35
+ Misses 4272 4235 -37
+ Partials 404 398 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 3f2e9b9 | Previous: 030010c | Ratio |
|---|---|---|---|
Benchmark_DecryptString |
6253 ns/op 2032 B/op 16 allocs/op |
2034 ns/op 2032 B/op 16 allocs/op |
3.07 |
Benchmark_DecryptString - ns/op |
6253 ns/op |
2034 ns/op |
3.07 |
This comment was automatically generated by workflow using github-action-benchmark.
almas-x
force-pushed
the
almas/chore
branch
3 times, most recently
from
87acc99 to
b0d77bc
Compare
| facadesSet = make(map[string]struct{}) | ||
| ) | ||
|
|
||
| r.bindings.Range(func(key, value interface{}) bool { |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the facade installation and uninstalling logic to address an issue where previously uninstalled facades could still be compiled into the binary. The refactor centralizes facade dependency information into a single source of truth and dynamically retrieves dependencies, ensuring that removed facades are properly excluded from the build process.
- Centralized facade metadata into
contracts/binding/facades.gowith a unifiedFacadeInfostructure - Updated service providers to reference dependencies from the centralized facade registry
- Modified package install/uninstall commands to use the new facade system
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/binding/facades.go | Introduces centralized facade metadata structure with package paths, dependencies, and base facade flags |
| foundation/facades.go | Removes legacy facade management code and helper functions |
| foundation/console/package_*_command.go | Updates install/uninstall commands to use new facade system with binding-based naming |
| support/collect/collect.go | Enhances Unique function to accept multiple collections for merging |
| */service_provider.go | Updates all service providers to reference dependencies from centralized facade registry |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| }{ | ||
| { | ||
| name: "not found binding", | ||
| name: "not found Binding", |
There was a problem hiding this comment.
The test name should use lowercase 'binding' for consistency with other test names and standard naming conventions.
| name: "not found Binding", | |
| name: "not found binding", |
| Validation: { | ||
| PkgPath: "github.com/goravel/framework/validation", |
There was a problem hiding this comment.
The Validation facade is missing a Dependencies field. For consistency with other facades, consider explicitly setting Dependencies: []string{} or adding a comment explaining why it has no dependencies.
| Validation: { | |
| PkgPath: "github.com/goravel/framework/validation", | |
| PkgPath: "github.com/goravel/framework/validation", | |
| Dependencies: []string{}, |
📑 Description
Closes goravel/goravel#760
This PR refactors the logic for installing facades, addressing an issue where previously uninstalled facades could still be compiled into the binary. By restructuring the facade installation workflow, the build process now correctly excludes facades that have been removed, ensuring a cleaner and more accurate binary output.
Before:
After:
✅ Checks