You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "DomGarguilo (via GitHub)" <gi...@apache.org> on 2023/03/30 16:47:10 UTC
[GitHub] [accumulo] DomGarguilo opened a new pull request, #3268: Fix TableOperations.onDemand() javadoc
DomGarguilo opened a new pull request, #3268:
URL: https://github.com/apache/accumulo/pull/3268
Fixes typos
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on pull request #3268: Fix TableOperations.onDemand() javadoc
Posted by "DomGarguilo (via GitHub)" <gi...@apache.org>.
DomGarguilo commented on PR #3268:
URL: https://github.com/apache/accumulo/pull/3268#issuecomment-1492586116
The original intent of this PR was to fix the typos in TableOperations from "online" to "on-demand".
In the latest commit I took a quick look through the comments to see where I could apply the suggestions made by @ctubbsii.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #3268: Fix TableOperations.onDemand() javadoc
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on PR #3268:
URL: https://github.com/apache/accumulo/pull/3268#issuecomment-1492476789
I think "on-demand" should be stylized with a hyphen when used as an adjective in prose/documentation. Variables/constants can be `ON_DEMAND`, or `ONDEMAND`, or `onDemand`, as needed, but when writing in English and not referring to the specific variable, the hyphenated form is more grammatically correct.
For what it's worth, https://en.wikipedia.org/wiki/On-demand uses the hyphenated form as its disambiguation page for the term. As an adjective, "on-demand" seems correct. However, as a prepositional phrase, "on demand" seems correct. So, an "on-demand table" is a "table [made available] on demand".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] Manno15 commented on a diff in pull request #3268: Fix TableOperations.onDemand() javadoc
Posted by "Manno15 (via GitHub)" <gi...@apache.org>.
Manno15 commented on code in PR #3268:
URL: https://github.com/apache/accumulo/pull/3268#discussion_r1155120201
##########
core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java:
##########
@@ -1025,10 +1025,10 @@ void onDemand(String tableName)
throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
/**
- * Initiates setting a table to ondemand state, optionally waits for action to complete
+ * Initiates setting a table to on-demand state, optionally waits for action to complete
Review Comment:
Same as above
##########
core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java:
##########
@@ -1025,10 +1025,10 @@ void onDemand(String tableName)
throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
/**
- * Initiates setting a table to ondemand state, optionally waits for action to complete
+ * Initiates setting a table to on-demand state, optionally waits for action to complete
*
- * @param tableName the table to take online
- * @param wait if true, then will not return until table is online
+ * @param tableName the table to set to on-demand state
+ * @param wait if true, then will not return until table state is set to on-demand state
Review Comment:
Same as above.
##########
core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java:
##########
@@ -1037,12 +1037,12 @@ void onDemand(String tableName, boolean wait)
throws AccumuloSecurityException, AccumuloException, TableNotFoundException;
/**
- * Check if a table is ondemand through its current goal state only. Could run into issues if the
+ * Check if a table is on-demand through its current goal state only. Could run into issues if the
* current state of the table is in between states. If you require a specific state, call
- * <code>ondemand(tableName, true)</code>, this will wait until the table reaches the desired
+ * <code>onDemand(tableName, true)</code>, this will wait until the table reaches the desired
* state before proceeding.
*
- * @param tableName the table to check if online
+ * @param tableName the table to check if in on-demand state
Review Comment:
Same as above.
##########
core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java:
##########
@@ -1014,9 +1014,9 @@ default TimeType getTimeType(String tableName) throws TableNotFoundException {
}
/**
- * Initiates setting a table to ondemand state, but does not wait for action to complete
+ * Initiates setting a table to on-demand state, but does not wait for action to complete
Review Comment:
Small grammar things here. This could read better if it's setting a table to 'an' or maybe 'the' on-demand state ('an' for better grammar or 'the' to specify that it is a unique table state in accumulo).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo merged pull request #3268: Fix TableOperations.onDemand() javadoc
Posted by "DomGarguilo (via GitHub)" <gi...@apache.org>.
DomGarguilo merged PR #3268:
URL: https://github.com/apache/accumulo/pull/3268
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org