commons-lang
commons-lang copied to clipboard
apache
Lang-1657: Diff Result Type Constraint
- Updated type parameter for DiffBuilder.append(String, DiffResult<T>) 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
Coverage increased (+0.006%) to 94.991% when pulling 24e8f735272c915123cf58c9469fcfc2867c626d on greatmastermario:LANG-1657-DiffResult-Type-Constraint into ce477d9140f1439c44c7a852d7df1e069e21cb85 on apache:master.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
4fc2ab4) 92.15% compared to head (5dd709c) 92.16%. Report is 15 commits behind head on master.
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@greatmastermario What happened to this pull request? This change is really needed. Having the DiffBuilder<T>.append() constraint on passing a DiffResult<T> is not ideal. Ideally you want to recursively call on nested attributes that implement the Diffable interface. I am not getting build errors but a lot of Uncheck warnings which are unpleasant.
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 https://github.com/apache/commons-lang/pull/786#issuecomment-1637173471, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD6TON4V3CUY3MDTTRZCKHLXQQ7QJANCNFSM5BXA2TBQ . You are receiving this because you were mentioned.Message ID: @.***>
@greatmastermario
I'm having trouble applying the patch locally on master with git apply. Would you rebase on master, please?
@greatmastermario I'm having trouble applying the patch locally on master with
git apply. Would you rebase on master, please?
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 https://github.com/apache/commons-lang/pull/786#discussion_r1264740474
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) https://github.com/apache/commons-lang/pull/786#discussion_r1264740474
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 https://github.com/apache/commons-lang/pull/786#issuecomment-1638297185, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD6TON4K4JZROZKHSQBITXDXQVFWLANCNFSM5BXA2TBQ . You are receiving this because you were mentioned.Message ID: @.***>
@greatmastermario Don't worry about git for now since GitHub does seem to see conflicts for a merge.
@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.