ThisaraWeerakoon commented on code in PR #826:
URL: https://github.com/apache/dubbo-go-samples/pull/826#discussion_r2041130241
##########
llm/config/config.go:
##########
@@ -94,6 +95,18 @@ func Load(envFile string) (*Config, error) {
return
}
config.NacosURL = nacosURL
+ maxContextStr := os.Getenv("MAX_CONTEXT_COUNT")
+ defaultMaxContextCount := 3 // Default to 3 for backward
compatibility
+ if maxContextStr == "" {
+ config.MaxContextCount = defaultMaxContextCount
+ return
Review Comment:
I see your point — returning early when maxContextStr == "" would indeed
cause the rest of the config fields to be skipped, which isn't ideal.
What I had in mind was something like this:
> ```go
> if maxContextStr == "" {
> config.MaxContextCount = defaultMaxContextCount
> } else {
> maxContext, err := strconv.Atoi(maxContextStr)
> if err != nil {
> configErr = fmt.Errorf("invalid MAX_CONTEXT_COUNT
value: %v", err)
> return
> }
> config.MaxContextCount = maxContext
> }
> ```
This avoids the early return and ensures the rest of the config continues to
be processed. That said, adding an `else` block introduces some extra nesting,
which might affect readability.
Do you think it’s worth refactoring to reduce the nesting (e.g., by
inverting the condition), or would you prefer to keep it like this for clarity?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
