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

OAK-11078: Remove usage of Guava checkArgument #1687

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft

Conversation

reschke
Copy link
Contributor

@reschke reschke commented Sep 4, 2024

Before proceeding with this, I'd love to get some feedback.

JDK lacks this feature. We can either use something identical from commons-lang (which we use anyway, and which I did here), or roll our own.

Opinions?

(note that I also took the freedom to fix "{}" in format strings, and to avoid string concatenation)

@@ -244,8 +244,8 @@ public static String checkValidName(String asyncName){
if (IndexConstants.ASYNC_REINDEX_VALUE.equals(asyncName)){
return asyncName;
}
checkArgument(asyncName.endsWith("async"), "async name [%s] does not confirm to " +
"naming pattern of ending with 'async'", asyncName);
Validate.isTrue(asyncName.endsWith("async"), "async name [%s] does not confirm to naming pattern of ending with 'async'",
Copy link
Contributor

Choose a reason for hiding this comment

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

s/confirm/conform

@@ -700,7 +700,7 @@ private static final class TreeNode {
}

TreeNode(MapFactory mapFactory, TreeNode parent, String name) {
checkArgument(!name.contains("/"),
Validate.isTrue(!name.contains("/"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does "{}" even work in a format string?

Copy link
Contributor Author

@reschke reschke Sep 6, 2024

Choose a reason for hiding this comment

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

No, according to the documentation (even in checkArgument)

so yes, this needs to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants