fix: use idiomatic Go boolean check in singleton.go#322
Conversation
Replace `if exists == false` with `if !exists` per Go style conventions. The Go Code Review Comments wiki recommends against comparing boolean values to false explicitly. Signed-off-by: HarshitPal25 <harshit13082006@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR performs a small cleanup in the store singleton initialization logic by using idiomatic Go boolean negation when checking the result of os.LookupEnv, aligning with common Go linting recommendations.
Changes:
- Replace
if exists == falsewithif !existsinpkg/store/singleton.go.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the environment variable check in pkg/store/singleton.go to use idiomatic Go boolean syntax. The review feedback suggests further improving this logic by also handling cases where the environment variable is set to an empty string to ensure the default storage provider is correctly initialized. Additionally, the reviewer recommended applying this idiomatic cleanup consistently across other files in the repository.
| // Setting storage provider type by env STORE_TYPE | ||
| providerType, exists := os.LookupEnv("STORE_TYPE") | ||
| if exists == false { | ||
| if !exists { |
There was a problem hiding this comment.
While this change correctly makes the boolean check idiomatic, it is also recommended to handle the case where the environment variable is set but empty (e.g., STORE_TYPE=""). In many deployment scenarios, an empty string is intended to trigger the default behavior. Additionally, please consider applying this same idiomatic boolean check cleanup to other files in this repository to ensure consistency, as several other instances exist (e.g., pkg/store/store_redis.go:210, pkg/store/store_valkey.go:82, and pkg/store/store_valkey.go:90).
| if !exists { | |
| if !exists || providerType == "" { |
References
- The Go Code Review Comments wiki recommends against comparing boolean values to true or false explicitly. It is also a best practice to handle empty environment variables as 'not set' when providing defaults. (link)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #322 +/- ##
==========================================
+ Coverage 47.57% 47.74% +0.17%
==========================================
Files 30 30
Lines 2819 2855 +36
==========================================
+ Hits 1341 1363 +22
- Misses 1338 1344 +6
- Partials 140 148 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description:
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Replaces
if exists == falsewithif !existsinsingleton.go. The Go Code Review Comments wiki andgolint/revivelinters recommend against comparing boolean values totrue/falseexplicitly.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: