-
Notifications
You must be signed in to change notification settings - Fork 305
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
Fix a lot of naming convention violations #5256
Fix a lot of naming convention violations #5256
Conversation
Preparing for the updated errorprone rules to come in so fixing problems. Note we suppress the errors for beacon rest API definition classes.
@Test | ||
public void equals_shouldReturnTrue() { | ||
final T state = randomState(); | ||
final T stateCopy = (T) state.updated(__ -> {}); | ||
final Object stateCopy = state.getSchema().createFromBackingNode(state.getBackingNode()); | ||
|
||
assertThat(state == stateCopy).isFalse(); | ||
assertThat(state).isEqualTo(stateCopy); | ||
assertThat(state.hashCode()).isEqualTo(stateCopy.hashCode()); | ||
} | ||
|
||
@SuppressWarnings("unchecked") | ||
@Test | ||
public void equals_shouldReturnFalse() { |
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.
Nice catch. I guess these weren't being tested before.
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.
Yeah I was pleased to catch that. They aren't particularly important tests but still nice to fix.
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
@@ -152,7 +152,7 @@ public SignedBlockAndState finalizeCurrentChain() { | |||
saveBlock(newChainHead); | |||
updateBestBlock(newChainHead); | |||
} | |||
final Checkpoint finalizedCheckpoint = newChainHead.getState().getFinalized_checkpoint(); | |||
final Checkpoint finalizedCheckpoint = newChainHead.getState().getFinalizedCheckpoint(); |
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.
GitHub code scanning correctly pointed out that newChainHead
could be null. But this is just a test fixture and those values are hardcoded, so it doesn't matter.
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.
Fortunately it has "Used in tests" and "Won't fix" options to make it be quiet. :)
PR Description
Preparing for the updated errorprone rules to come in so fixing problems. Note we suppress the errors for beacon rest API definition classes.
Also fixes a few cases which violate the new
MathTargetType
check (Consensys/errorprone-checks#7).Documentation
doc-change-required
label to this PR if updates are required.Changelog