-
Notifications
You must be signed in to change notification settings - Fork 19
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
(Sentinel) Fix dal stats chcker, struct properties should be public to be parsed through pgx #2226
Conversation
WalkthroughWalkthroughThe changes involve modifications to the SQL query and updates to the data structures in the application. The SQL constant Changes
Sequence Diagram(s)sequenceDiagram
participant DB as Database
participant App as Application
participant Key as Key Struct
participant Count as Count Struct
App->>DB: Request GetAllValidKeys
DB-->>App: Return Key, Description
App->>Key: Initialize Key struct
App->>Count: Initialize Count struct
App->>Key: Access Key.Field
App->>Count: Access Count.Field
Poem
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 selected for processing (1)
- node/pkg/checker/dalstats/app.go (2 hunks)
Additional comments not posted (6)
node/pkg/checker/dalstats/app.go (6)
18-18
: LGTM!The SQL query now selects specific columns, which improves performance and clarity by only fetching necessary fields.
The code changes are approved.
26-26
: LGTM!The
DBKey
struct fieldkey
has been renamed toKey
to follow Go's exported naming conventions, enhancing visibility and accessibility.The code changes are approved.
27-27
: LGTM!The
DBKey
struct fielddescription
has been renamed toDescription
to follow Go's exported naming conventions, enhancing visibility and accessibility.The code changes are approved.
31-31
: LGTM!The
Count
struct fieldcount
has been renamed toCount
to follow Go's exported naming conventions, enhancing visibility and accessibility.The code changes are approved.
81-81
: LGTM!The code now references
key.Description
instead of the previous unexported counterpart, reflecting the updated struct field name and ensuring proper access.The code changes are approved.
85-101
: LGTM!The code now references
key.Key
instead of the previous unexported counterpart, and the message formatting at the end of the function has been adjusted to accommodate the new field names. These changes reflect the updated struct field names and ensure proper access.The code changes are approved.
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.
good catch 🙏
Description
Found out that the struct properties weren't setup as public leading to query failure
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment