-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Generic bindings #3626
base: master
Are you sure you want to change the base?
Generic bindings #3626
Conversation
WalkthroughThe recent changes focus on improving the codebase's clarity and organization within the binding mechanism. Key updates include dependency upgrades for better security and performance, simplifications in the method binding logic, and the introduction of a structured approach to handle method paths. These modifications enhance maintainability and lay the groundwork for potential feature expansions, including support for binding generic structs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Binding
participant Database
Client->>Binding: Add method
Binding->>Database: AddMethod(DecodedMethod)
Database-->>Binding: Success
Binding-->>Client: Confirmation
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
v2/examples/customlayout/go.sum
is excluded by!**/*.sum
Files selected for processing (4)
- v2/examples/customlayout/go.mod (1 hunks)
- v2/internal/binding/binding.go (2 hunks)
- v2/internal/binding/boundMethod.go (3 hunks)
- v2/internal/binding/reflect.go (3 hunks)
Additional comments not posted (13)
v2/examples/customlayout/go.mod (4)
32-32
: Update: Dependencygolang.org/x/crypto
upgraded tov0.23.0
.The upgrade from
v0.17.0
tov0.23.0
may include security patches and performance improvements. Ensure to test the application for compatibility.
34-34
: Update: Dependencygolang.org/x/net
upgraded tov0.25.0
.The upgrade from
v0.17.0
tov0.25.0
may include security patches and performance improvements. Ensure to test the application for compatibility.
35-35
: Update: Dependencygolang.org/x/sys
upgraded tov0.20.0
.The upgrade from
v0.15.0
tov0.20.0
may include security patches and performance improvements. Ensure to test the application for compatibility.
36-36
: Update: Dependencygolang.org/x/text
upgraded tov0.15.0
.The upgrade from
v0.14.0
tov0.15.0
may include security patches and performance improvements. Ensure to test the application for compatibility.v2/internal/binding/boundMethod.go (5)
9-13
: New Struct:BoundedMethodPath
.The new struct encapsulates the package, struct, and method names, providing a structured way to represent method paths. This improves the organization and readability of method-related data.
15-17
: New Method:FullName
forBoundedMethodPath
.The
FullName
method returns the full name of the method in the formatPackage.Struct.Name
. This enhances error reporting by providing more informative output.
22-22
: Integration:Path
field inBoundMethod
.The
Path
field of type*BoundedMethodPath
is added to theBoundMethod
struct. This integration improves the organization and readability of method-related data.
43-43
: Improvement: Enhanced error message inParseArgs
.The error message now includes the full method name using
b.Path.FullName()
, providing more informative output regarding method identification.
67-67
: Improvement: Enhanced error message inCall
.The error message now includes the full method name using
b.Path.FullName()
, providing more informative output regarding method identification.v2/internal/binding/reflect.go (3)
27-45
: New Helper Function:normalizeStructName
.The function standardizes the naming of struct types by replacing certain characters with more suitable alternatives. This helps in maintaining consistent naming conventions.
70-71
: Modification: UsenormalizeStructName
ingetMethods
.The
structName
is now normalized usingnormalizeStructName
, ensuring consistent naming conventions for struct types.
87-91
: Integration: UseBoundedMethodPath
ingetMethods
.The
getMethods
function now constructs method paths usingBoundedMethodPath
, improving the clarity and maintainability of the code.v2/internal/binding/binding.go (1)
77-77
: LGTM! The simplification enhances readability.The changes reduce unnecessary complexity by directly using the
method.Path
structure. Ensure that themethod.Path
structure is correctly populated in all cases.
Description
Enable generic structs to be added as binds to the application. It also simplifies how the struct names are being broken, and removes the necessity of splitting the struct name multiple times. Before those changes we can see from the code that the struct name has already been broken into its components, but then it is glued together, later when adding the method into the binding we are doing the reverse, thus we are adding complexity that is not needed.
Fixes #3625
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor
.Test Configuration
Checklist:
website/src/pages/changelog.mdx
with details of this PRThe ones I have skipped I deemed non-related as the changes made were quite minimal. Can do so if deemed necessary by maintainers.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor