Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor(core,server): make commands not rely on server context #20422
refactor(core,server): make commands not rely on server context #20422
Changes from 22 commits
7a6f8aa
620509e
67e13d1
16c293b
217c539
c46359d
251fa0c
95b2c29
467428e
b9a7278
4dcf9e6
e8d9242
5e1df8d
77c3bc1
081cc22
0b8808e
6c9992d
f63a54e
ef68e88
eafb6bb
cc3a5df
e7aee2d
a360be7
1d3c29f
c27b303
0bcea5e
6ab5b56
b31896d
7c4e602
baae5d4
5de4f6c
9fbb8e8
731751b
d7e7864
c90c6ac
e3c0a03
7896c75
9fb1cf8
8eb2ebf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Ensure proper error handling when retrieving Viper instance from command context.
The function
GetViperFromCmd
returns a new Viper instance if it cannot find one in the command context. This might lead to unexpected behavior if the context was supposed to already contain a specific configuration. Consider throwing an error or logging a warning when the Viper instance is not found.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.
Optimize error handling in
GetConfigFromViper
.The function
GetConfigFromViper
handles errors by returning a default configuration with the root directory set. However, it might be more appropriate to also log the error or throw it back to the caller to handle it, as swallowing the error might hide underlying issues.Check failure on line 46 in server/cmd/execute.go
GitHub Actions / golangci-lint
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.
Context key should be a custom type, not a built-in type.
Using built-in types as context keys can lead to collisions. Define a custom type to use as a key to avoid this issue.
Check failure on line 47 in server/cmd/execute.go
GitHub Actions / golangci-lint
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.
Context key should be a custom type, not a built-in type.
Similar to the previous comment, use a custom type for the context key to prevent potential collisions.
Committable suggestion
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.
Use custom types for context keys to avoid potential collisions.
Using built-in types as context keys can lead to collisions. Define a custom type to use as a key to avoid this issue.
Committable suggestion