-
-
Notifications
You must be signed in to change notification settings - Fork 274
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
Add new WithSliceDeepMerge config option #180
Conversation
} else { | ||
if err = deepMerge(dstElem, srcElem, visited, depth+1, config); err != nil { | ||
return | ||
} else { |
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.
if block ends with a return statement, so drop this else and outdent its block
} else { | ||
if err = deepMerge(dstElem, srcElem, visited, depth+1, config); err != nil { | ||
return | ||
} else { |
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.
if block ends with a return statement, so drop this else and outdent its block
Hello, @arnaudbriche! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information. |
SourceLevel has finished reviewing this Pull Request and has found:
|
@@ -231,6 +266,39 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co | |||
return | |||
} | |||
} | |||
} else if sliceDeepMerge { |
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.
Avoid deeply nested control flow statements.
@arnaudbriche Check if #210 fixes your use case. |
Hi!
I tested this and didn't work as expected for the proposed feature. I also tested the code in this PR and were able to make it work for my use case. Would it be possible to reopen this PR? Perhaps another PR should be opened? If needed, I can work on whatever was blocking this PR to be merged. |
@drenizg Sure, don't hesitate to work on this. Thanks! I think you'll need to reopen a new PR, as this one is not possible. The original repo was closed. |
Added validations to some reflection calls that were causing panic: "reflect: call of reflect.Value.IsZero on zero Value"
No description provided.