-
-
Notifications
You must be signed in to change notification settings - Fork 420
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
fix: generate config returns after logging error #1154
Conversation
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Apply Sweep Rules to your PR?
|
Hey @shivamsouravjha, Can you please review this PR? |
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.
Try installing go extension and fix the code style. The return terminology isn't right(set code style update)
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Hey @shivamsouravjha, can you please review this? |
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.
-
Quick note on the logging strategy in the code: When we use a logger at the 'fatal' level, the code execution halts automatically at that point. Hence, adding return statements after each 'fatal' logger isn't necessary.
-
Also, on choosing between 'Error' and 'Fatal' for logging: 'Fatal' should be used in situations where the application must stop immediately due to a critical issue. This ensures immediate attention to severe problems and prevents the continuation of processes that could be harmful
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
@officialasishkumar are you working on this issue |
I've made the necessary changes. Please take a look @shivamsouravjha |
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.
@officialasishkumar reformat the code as with a reformat only change is the return statement. Are you using vscode or goland for this?
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.
reformat the code as formatting isssue
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
b0d8731
to
318ad3c
Compare
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
@shivamsouravjha PTAL |
@officialasishkumar congratulations on the PR merge! |
Related Issue
The implementation of
keploy/pkg/service/generateConfig/generateConfig.go
doesn't return in case of error and is subject to nil memory exception.Closes: #1152
Describe the changes you've made
Added return
Type of change
Please let us know if any test cases are added
Please describe the tests(if any). Provide instructions how its affecting the coverage.
Describe if there is any unusual behaviour of your code(Write
NA
if there isn't)NA
A clear and concise description of it.
Checklist:
Screenshots (if any)