Skip to content
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.

Sign up for GitHub

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

Jump to bottom

95 Add stable equals/hashCode for PBJ objects #109

Merged
merged 10 commits into from
Nov 22, 2023
Merged

95 Add stable equals/hashCode for PBJ objects #109

merged 10 commits into from
Nov 22, 2023

Conversation

imalygin
Copy link
Member

@imalygin imalygin commented Nov 7, 2023

This pull request is a cleaner version of #102

I excluded formatting and fixed some minor warnings.

Related issue(s):

Fixes #95

Notes for reviewer:

This implementation does not include optional fields with default values into hashCode / equals calculations, so changing the object definition is not going to affect how existing objects behave.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 7, 2023 •
edited
Loading

JUnit Test Report

     39 files  ±0       39 suites  ±0   2m 8s ⏱️ -58s
   504 tests  - 1     503 ✔️  - 1  1 💤 ±0  0 ❌ ±0 
2 692 runs   - 1  2 683 ✔️  - 1  9 💤 ±0  0 ❌ ±0 

Results for commit df696c8. ± Comparison against base commit 3e79908.

This pull request removes 3 and adds 1 tests. Note that renamed tests count towards both. 
, 1
com.hedera.pbj.runtime.Utf8ToolsTest ‑ [4] 
com.hedera.pbj.runtime.io.buffer.BufferedDataTest ‑ readBytes() does always copy

com.hedera.pbj.runtime.Utf8ToolsTest ‑ [4] 
, 1

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 7, 2023 •
edited
Loading

Integration Test Report

86 963 tests  +242   86 963 ✔️ +242   4m 32s ⏱️ -1s
     213 suites +    4            0 💤 ±    0 
     213 files   +    4            0 ❌ ±    0 

Results for commit df696c8. ± Comparison against base commit 3e79908.

♻️ This comment has been updated with latest results.

@imalygin imalygin force-pushed the 095-hashcode branch 3 times, most recently from a4a066d to f73ef6c Compare November 13, 2023 21:03
@codacy-production Codacy Production
Copy link
Contributor

codacy-production bot commented Nov 14, 2023 •
edited
Loading

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
❌ -1.15% (target: -1.00%) 0.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (3e79908) 3611 1013 28.05%
Head commit (df696c8) 3766 (+155) 1013 (+0) 26.90% (-1.15%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#109) 262 0 0.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

artemananiev
artemananiev previously approved these changes Nov 16, 2023
View reviewed changes
Lubomir Litchev and others added 7 commits November 20, 2023 17:03
@imalygin
Implement stable hashCode for record fields.
6becef8 
Cleaned up some code.
Add equals method implementation.
Added tests and benchmarks for hashCode and equals

This is a fix for issue #95.

Signed-off-by: Lubomir Litchev <lubomir@swirldslabs.com>
Co-authored-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Clean up
ee7e91e 
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Fixed generated code indentation. Removed code duplication. Replaced …
abb4574 
…adding spaced with `.indent` calls.

Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Addressed Artem's comment.
dfa400b 
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin @codacy-production
Apply suggestions from code review
7c69529 
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com>
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Addressed review comments.
921e6c6 
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Added patch from Jasper. Addressed Jasper's comments. Changed generat…
e21de96 
…ed code directory layout to match Maven.

Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Addressed review comments.
cfb554d 
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Addressed review comments.
52376d0 
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin
Addressed review comments.
df696c8 
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin imalygin merged commit 9238011 into main Nov 22, 2023
8 of 10 checks passed
@imalygin imalygin deleted the 095-hashcode branch November 22, 2023 16:23
This was referenced Nov 22, 2023
Merged
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codacy-production codacy-production[bot] codacy-production[bot] left review comments

@artemananiev artemananiev artemananiev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Generated PBJ objects should have proper hashCode() implementation
2 participants
@imalygin @artemananiev