Issue 32 (do not merge, full integration/regression tests pending) #33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rwrite a members type with a primitive if it's currently Object, additionally, when encountering a member during a merge operation that is an ApexClass we recursively call merge on all of that ApexClass' members.
… for the fact that those members might also contain lists that need to be merged and recursed on. (WIP)
…writing Object primitives with complex types it's possible that we have information about a complex member but are only merging in the direction that will cause us to hold on to our primitive.
…that it does a deep comparison of all members and all of their members members etc.
josephbleau
changed the title
|
DO NOT MERGE. Integration/regression tests pending, opened for review and discussion. |
|
Thanks for looking into this!. I haven't looked at this code in a long time, so its going to take me a while to go through the changes. |
|
No worries! I want to spend more time with it. I started feeling a bit guilty to be honest that I sort of dumped all of this out of nowhere and given the codebase is totally new to me it’s completely possible that I’ve missed important things. If you’d rather not spend the energy please don’t feel obligated! |
added 2 commits
February 24, 2020 13:48
…ve, ApexClass, and ApexList. The missing component was the fact that an ApexPrimitive.OBJECT type should now be considered "equal" to all other types. Effectively, it's being used as an "UnknownType" property, and because equality is used to determine "same type"-edness and overwriting in merges it must be equal to all other types. I am going to probably use a different name overall in a upcoming change. ApexPrimitive.UnknownType is a good candidate.
…hen encountered in a list together), as two types may be equal by the fact that one of their members has the same name as the other, but is of UnknownType (Object) and the other is already mapped. When this happens we still want to merge them, and so equality cannot be used to prefilter mergable types. We should implement a method that determines "true equality" aka "all members are the same name, and the same type" as an optimization, but merging always is not a huge performance problem... for me... Lastly, for simmilar reasons, remove member list from hash value of ApexClass as we were getting duplicates in the type set (same class, but one with an UnknownType member and another which was resolved.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Addresses the concerns outlined in #32