Patch / Merge

[ghost]

My first attempt at a Patch / Merge implementation for Diff's change log format.

Folded into diff's source tree. Features include:

• Patch function which takes a change log and a pointer to a target
• Merge function which takes origin and changed values as well as a pointer to a target
• produces a PatchLog, similar to a ChangeLog in which what action was taken is described
• Offers a couple of struct tag additions that affect how patch applies changes, these include:
  • omitunequal, which applies the strict rule that the target value must match the origin value or it will not get replaced
  • create, which allows for the creation of map and slice elements should they not already be present
• Patch honors the "-" name in the diff tag's name portion. thus excluding said strut member from updates
• Patch honors the diff struct tags 'name' and will apply the change to the target member or map appropriately.
• There are examples in diff_examples-_test.go These help maintain the code bases unit test coverage percentage
• There is a new error utility, which allows for nested link lists of errors in a special type.
• The business of patching is fuzzy and errors don't necessarily mean the operation failed so patch is a 'best effort' operation and will fulfill all the changes in the change lot to the target as best it can. Audit / errors are contained in the PatchLog

Test pass and code coverage remains above 80%

A simple usage example:

[purehyperbole]

Hey, thanks for opening the PR.

At a first glance, all the changes look really good. Awesome work! :smile:

I'll try and do a deeper review of the changes this evening. The only things questions I have so far are:

1. Are the wrapping recover() calls necessary when calling SetFlag()? It's not immediately obvious to me why that bitwise or operation would (or even could?) panic and you are already checking if the changelog value is nil before setting anything. I may be missing something completely though!

2. Could the error wrapping functionality that released in go 1.13 save having to write the additional code to wrap errors (I haven't actually used that new functionality though, so it may not be a great fit)

Thanks again! This looks like a really well thought out and implemented feature

[ghost]

Hey, thanks for opening the PR.

At a first glance, all the changes look really good. Awesome work! 😄

I'll try and do a deeper review of the changes this evening. The only things questions I have so far are:

1. Are the wrapping recover() calls necessary when calling SetFlag()? It's not immediately obvious to me why that bitwise or operation would (or even could?) panic and you are already checking if the changelog value is nil before setting anything. I may be missing something completely though!
2. Could the error wrapping functionality that released in go 1.13 save having to write the additional code to wrap errors (I haven't actually used that new functionality though, so it may not be a great fit)

Thanks again! This looks like a really well thought out and implemented feature

So we're not really wrapping SetFlag here, we're deferring recover in the event that reflect.Set goes wrong on lines 113 & line 126.

So here's the thing. reflect.Set doesn't return an error, but panics if something goes wrong, and there's a lot that can go wrong there including:

• Trying to set to an unsafe pointer which tosses a panic of segfault
• Trying to set a value that isn't settable (i.e. a passed by value instead of a pass by ref)
• Other minutia

An example case in which we could panic would be; a.ID and b.ID are integers and c.ID is a GUID

set will panic at the error since we only type check for slice and map… even then key types could be different.

If we allow the panic, we're either telling the consumer to handle it or we need to pass an error back (which doesn't fit the decorator pattern as we're echoing the functionality) either way we’re failing fast and aborting the operation to inform the consumer and ask them to handle the situation.

At the end of the day, I choose to fail and follow through, not fail fast as I believe the patch feature to be best effort, in other words I want it to always succeed, report the failure but continue on and complete the remaining tasks that may be valid.

For example, say origin a and diff b produce change log c applied to a similar but different struct d. Some, if not most of c can be applied and the result may still be quite valid (think polymorphism here). In this manor, we report the failure but still follow through, it’s not necessary to generate an error here because with flag each operation with a series of indicators as to the status of the particular change log operation.

[ghost]

On question 2, honestly I've been wrapping errors so long in Go that I'm sure I noticed in 1.13 that wrapping was introduced however it's been almost automatic to convert errors to the pattern I introduced in the pull request. I read the blog post when you mentioned it and did add support for Unwrap() to it this morning for compatibility. I didn't do anything more since the tail recursion pattern Im using for nesting isn't quite the same in the implementation I've come up with and would cause more pervasive changes to the patch code and honestly, I don't really want to revisit the logic as it's quite complicated.

Thanks for opening my eyes to it though, I'm hoping my implementation is idiomatic enough for the powers that be.

[purehyperbole]

Sorry for the delay in getting back to you.

Thank you for clarifying those questions and for adding the wrap method. Overall this patch looks great. I'll merge these changes now and cut a release.

And thank you again for the contribution! :smile: