-
Notifications
You must be signed in to change notification settings - Fork 184
chore: 21078: StateLogger is no longer used and can be removed #21156
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.
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
Conversation
Signed-off-by: Artem Ananev <[email protected]>
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #21156 +/- ##
============================================
- Coverage 71.23% 71.21% -0.02%
+ Complexity 24259 24241 -18
============================================
Files 2661 2661
Lines 103448 103419 -29
Branches 10815 10815
============================================
- Hits 73691 73651 -40
- Misses 25723 25748 +25
+ Partials 4034 4020 -14
... and 16 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Neeharika-Sompalli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @artemananiev
platform-sdk/swirlds-state-api/src/main/java/com/swirlds/state/spi/ReadableKVStateBase.java
Show resolved
Hide resolved
...src/testFixtures/java/com/swirlds/state/test/fixtures/merkle/disk/BackedReadableKVState.java
Show resolved
Hide resolved
...src/testFixtures/java/com/swirlds/state/test/fixtures/merkle/disk/BackedWritableKVState.java
Show resolved
Hide resolved
...dk/swirlds-state-impl/src/main/java/com/swirlds/state/merkle/disk/OnDiskReadableKVState.java
Show resolved
Hide resolved
...swirlds-state-impl/src/main/java/com/swirlds/state/merkle/disk/OnDiskReadableQueueState.java
Show resolved
Hide resolved
...lds-state-impl/src/main/java/com/swirlds/state/merkle/disk/OnDiskReadableSingletonState.java
Show resolved
Hide resolved
...dk/swirlds-state-impl/src/main/java/com/swirlds/state/merkle/disk/OnDiskWritableKVState.java
Show resolved
Hide resolved
...swirlds-state-impl/src/main/java/com/swirlds/state/merkle/disk/OnDiskWritableQueueState.java
Show resolved
Hide resolved
...lds-state-impl/src/main/java/com/swirlds/state/merkle/disk/OnDiskWritableSingletonState.java
Show resolved
Hide resolved
...testFixtures/java/com/swirlds/state/test/fixtures/merkle/queue/BackedReadableQueueState.java
Show resolved
Hide resolved
...testFixtures/java/com/swirlds/state/test/fixtures/merkle/queue/BackedWritableQueueState.java
Show resolved
Hide resolved
...ures/java/com/swirlds/state/test/fixtures/merkle/singleton/BackedReadableSingletonState.java
Show resolved
Hide resolved
...ures/java/com/swirlds/state/test/fixtures/merkle/singleton/BackedWritableSingletonState.java
Show resolved
Hide resolved
thenswan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left many similar comments, as I went and double-checked each class (if there is a need in a label), but they are all about one change - remove label from State API completely, as it was used only in StateLogger.
thenswan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the discussion, we decided to make further changes in a follow up PR, to keep this one scoped to the relevant issue
Signed-off-by: Artem Ananev <[email protected]>
d9ab8c0
tinker-michaelj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tyvm @artemananiev !
andrewb1269
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve .github/workflows/support/citr/log4j2.xml
Fixes: #21078 Reviewed-by: Andrew Brandt <[email protected]>, Ivan Malygin <[email protected]>, Michael Tinker <[email protected]>, Nikita Lebedev <[email protected]>, Roger Barker <[email protected]> Signed-off-by: Artem Ananev <[email protected]>
Fix summary:
StateLoggeris removed, as well as all the state change loggins calls from State API.Fixes: #21078
Signed-off-by: Artem Ananev [email protected]