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.

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

Latest gradle-baseline #6021

Merged
merged 1 commit into from
May 3, 2022
Merged

Latest gradle-baseline #6021

merged 1 commit into from
May 3, 2022

Conversation

carterkozak
Copy link
Contributor

==COMMIT_MSG==
Latest gradle-baseline
==COMMIT_MSG==

Comment on lines -304 to +305
return (long) (Math.min(BACKOFF_COEFF * Math.pow(2, numberOfAttempts), MAX_EXPECTED_BACKOFF) * randomCoeff);
return (long) (Math.min(BACKOFF_COEFF * Math.pow(2, numberOfAttempts), (double) MAX_EXPECTED_BACKOFF)
* randomCoeff);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From ./gradlew classes testClasses -PerrorProneApply=LongDoubleConversion

.numBytes(ESTIMATED_NUM_BYTES_PER_ROW * numberOfQueriedRows)
.numBytes(ESTIMATED_NUM_BYTES_PER_ROW * ((long) numberOfQueriedRows))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From ./gradlew classes testClasses -PerrorProneApply=NarrowCalculation

@@ -323,6 +323,7 @@ public Namespace getNamespace() {
*
* @param srcDir root source directory where code generation is performed.
*/
@SuppressWarnings("DangerousIdentityKey")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DangerousIdentityKey suppressions lifted directly from #5753

Comment on lines -75 to +74
log.debug(
"Encountered null metric context likely due to exception in preInvocation: {}",
UnsafeArg.of("cause", cause),
cause);
log.debug("Encountered null metric context likely due to exception in preInvocation", cause);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY: Logging the cause as a throwable provides everything that cause.toString does, and more!

Copy link
Contributor

@jeremyk-91 jeremyk-91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -94,7 +94,7 @@ public void queriesGaugesAgainAfterTimeInterval() {

value.set(8);
tick.addAndGet(DistributionOutlierController.REFRESH_INTERVAL.toNanos() + 1);
assertThat(defaultController.getMeanGauge().getValue()).isEqualTo(8L);
assertThat(defaultController.getMeanGauge().getValue()).isEqualTo((double) 8L);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertThat(defaultController.getMeanGauge().getValue()).isEqualTo((double) 8L);
assertThat(defaultController.getMeanGauge().getValue()).isEqualTo(8.0);

this is fine, and I don't expect us to fix this to get this merged, though this should really be 8.0 (possibly with whatever double equality hacks we need, though none are needed for 8).

More "the existing tests were naughty" rather than anything bad about this change

@@ -263,7 +263,7 @@ private void flushTask() {
}
} catch (Throwable t) {
if (!Thread.interrupted()) {
log.warn("Error occurred while flushing sweep stats: {}", UnsafeArg.of("throwable", t), t);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh. lol

@bulldozer-bot bulldozer-bot bot merged commit 59daf66 into develop May 3, 2022
@bulldozer-bot bulldozer-bot bot deleted the ckozak/baseline branch May 3, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants