Lang-1657: Diff Result Type Constraint #786
Conversation
greatmastermario
commented
Aug 7, 2021
greatmastermario
commented
- Updated type parameter for DiffBuilder.append(String, DiffResult) to accept DiffResults of any type in case nested field is not the same type as the containing class
- Added test case for nested Diffable types
|
May you please rebase on master to pick up the recent Java 16 fix. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #786 +/- ##
============================================
+ Coverage 92.15% 92.16% +0.01%
- Complexity 7596 7597 +1
============================================
Files 200 200
Lines 15910 15910
Branches 2925 2925
============================================
+ Hits 14662 14664 +2
+ Misses 675 674 -1
+ Partials 573 572 -1 ☔ View full report in Codecov by Sentry. |
|
@greatmastermario What happened to this pull request? This change is really needed. Having the |
|
Looking... |
|
Since it has been a bit since I opened this PR, let me know if any edits
are required.
…On Sun, Jul 16, 2023, 12:39 PM Gary Gregory ***@***.***> wrote:
Looking...
—
Reply to this email directly, view it on GitHub
<#786 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD6TON4V3CUY3MDTTRZCKHLXQQ7QJANCNFSM5BXA2TBQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@greatmastermario |
| @@ -495,4 +519,16 @@ public void testTriviallyEqualTestEnabled() { | |||
| assertThat(explicitTestAndEqual.build().getNumberOfDiffs(), equalToZero); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testNestedDiffable() { | |||
There was a problem hiding this comment.
This test needs to be different: It compiles just fine AND without warnings without the change to main. What am I missing?
There was a problem hiding this comment.
Let me validate again, but this should not be passing. The generic type T in master (did it get renamed to main? I don't see that in the repo) makes it where the passed diff needs to be the same type as the original builder. Making it ? allows other types of DiffBuilders to be passed. If this is not the behavior in master, I can rewrite the test.
There was a problem hiding this comment.
After checking out main/master and copy-pasting the test without the code fix, I am getting a compilation error due to the generic types, which is what was asked to be fixed in the original issue. The test cannot compile without the NestedTypeTestClass, so I am unsure why main would be working. Screenshot proof prior to the fix is attached.
Hi Gary, I am trying to do a git rebase, but it's considering every change from the last two years a "merge conflict" and making me manually apply all the changes. Should I close this PR, recreate the branch, and open a new PR? |
|
No, please don't open and close PRs and create noise. Just Google on how to use git and you'll get there, but for now, see my comment #786 (comment) If you can't figure out git, don't worry, and we will deal with the PR as it is. |
|
No problem, thought I would check. I typically use git pull to merge a
remote branch, so I'm not used to the rebase command. I can continue to
work through the rebate conflicts, was just trying to save some time.
…On Mon, Jul 17, 2023, 7:44 AM Gary Gregory ***@***.***> wrote:
No, please don't open and close PRs and create noise. Just Google on how
to use git and you'll get there, but for now, see my comment #786
(comment)
<#786 (comment)>
If you can't figure out git, don't worry, and we will deal with the PR as
it is.
—
Reply to this email directly, view it on GitHub
<#786 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD6TON4K4JZROZKHSQBITXDXQVFWLANCNFSM5BXA2TBQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@greatmastermario |
…esult<T>) to allow DiffResults that are not the same type as the original DiffBuilder
greatmastermario
force-pushed
the
LANG-1657-DiffResult-Type-Constraint
branch
from
8998881
to
5dd709c
Compare
|
@garydgregory Apologies for taking so long to get that rebase done. Finally have everything worked out with the rebase. Let me know if there are further changes required. |
