Incorporate waiters PR feedback

Author: mtdowling

docs/source/1.0/spec/waiters.rst

@@ -31,17 +31,14 @@ following client pseudocode:
 ``smithy.waiters#waitable`` trait
 =================================
 
-Waiters are defined on :ref:`operations <operation>` using the
-``smithy.waiters#waitable`` trait.
-
-Trait summary
+Summary
     Indicates that an operation has various named "waiters" that can be used
     to poll a resource until it enters a desired state.
 Trait selector
     ``operation :not(-[input, output]-> structure > member > union[trait|streaming])``
 
     (Operations that do not use :ref:`event streams <event-streams>` in their input or output)
-Trait value
+Value type
     A ``map`` of :ref:`waiter names <waiter-names>` to
     :ref:`Waiter structures <waiter-structure>`.
 
@@ -53,6 +50,8 @@ exists:
 
     namespace com.amazonaws.s3
 
+    use smithy.waiters#waitable
+
     @waitable(
         BucketExists: {
             documentation: "Wait until a bucket exists",
@@ -139,7 +138,7 @@ waiter implementations perform the following steps:
    3. Stop waiting if the acceptor transitions the waiter to the ``success``
       or ``failure`` state.
 
-4. If none of the acceptors are matched and an error was encountered while
+4. If none of the acceptors are matched *and* an error was encountered while
    calling the operation, then transition to the ``failure`` state and stop
    waiting.
 5. Transition the waiter to the ``retry`` state, follow the process
@@ -155,7 +154,7 @@ Waiter implementations MUST delay for a period of time before attempting a
 retry. The amount of time a waiter delays between retries is computed using
 `exponential backoff`_ through the following algorithm:
 
-* Let ``attempt`` be the number retry attempts.
+* Let ``attempt`` be the number of retry attempts.
 * Let ``minDelay`` be the minimum amount of time to delay between retries in
   seconds, specified by the ``minDelay`` property of a
   :ref:`waiter <waiter-structure>` with a default of 2.
@@ -182,8 +181,8 @@ or equal to ``minDelay``, then set ``delay`` to ``remainingTime`` minus
 needlessly only to exceed ``maxWaitTime`` before issuing a final request.
 
 Using the default ``minDelay`` of 2, ``maxDelay`` of 120, a ``maxWaitTime``
-of 300 (or 5 minutes), and assuming that requests complete in 0 seconds
-(for example purposes only), delays are computed as followed:
+of 300 (5 minutes), and assuming that requests complete in 0 seconds
+(for example purposes only), delays are computed as follows:
 
 .. list-table::
     :header-rows: 1
@@ -335,8 +334,8 @@ members MUST be set:
       - Matches on both the input and output of an operation using a JMESPath_
         expression. Input parameters are available through the top-level
         ``input`` field, and output data is available through the top-level
-        ``output`` field. This matcher can only be used on operations that
-        define both input and output. This matcher is checked only if an
+        ``output`` field. This matcher MUST NOT be used on operations that
+        do not define input or output. This matcher is checked only if an
         operation completes successfully.
     * - success
       - ``boolean``
@@ -488,9 +487,10 @@ comparator can be set to any of the following values:
         that is equal to an expected string.
       - ``string``
     * - booleanEquals
-      - Matches if the return value of a JMESPath expression is a boolean.
-        The ``expected`` value of a ``PathMatcher`` MUST be set to "true"
-        or "false" to match the corresponding boolean value.
+      - Matches if the return value of a JMESPath expression is a boolean
+        that is equal to an expected boolean. The ``expected`` value of a
+        ``PathMatcher`` MUST be set to "true" or "false" to match the
+        corresponding boolean value.
       - ``boolean``
     * - allStringEquals
       - Matches if the return value of a JMESPath expression is an array and
@@ -650,7 +650,7 @@ Implicit acceptors are unnecessary and can quickly become incomplete as new
 resource states and errors are added. Waiters have 2 implicit
 :ref:`acceptors <waiter-acceptor>`:
 
-* (Step 4) - If none of the acceptors are matched and an error was
+* (Step 4) - If none of the acceptors are matched *and* an error was
   encountered while calling the operation, then transition to the
   ``failure`` state and stop waiting.
 * (Step 5) - Transition the waiter to the ``retry`` state, follow the

docs/source/conf.py

@@ -28,7 +28,6 @@
 extensions = ['sphinx_tabs.tabs',
               # We use redirects to be able to change page names.
               'sphinxcontrib.redirects',
-              'sphinx.ext.imgmath',
               'smithy']
 
 # Add any paths that contain templates here, relative to this directory.

smithy-waiters/src/main/java/software/amazon/smithy/waiters/AcceptorState.java

@@ -32,7 +32,7 @@ public enum AcceptorState implements ToNode {
     /** Transition to a final failure state. */
     FAILURE,
 
-    /** Transition to a final retry state. */
+    /** Transition to an intermediate retry state. */
     RETRY;
 
     @Override

smithy-waiters/src/main/java/software/amazon/smithy/waiters/ModelRuntimeTypeGenerator.java

@@ -152,11 +152,10 @@ private Object createListOrSet(Shape shape, MemberShape member) {
         });
     }
 
-    // Visits members and mutates a copy of the current set of
-    // visited shapes rather than a shared set. This a shape to
-    // be used multiple times in the closure of a single shape
-    // without causing the reuse of the shape to always be
-    // assume to be a recursive type.
+    // Visits members and mutates a copy of the current set of visited
+    // shapes rather than a shared set. This allows a shape to be used
+    // multiple times in the closure of a single shape without causing the
+    // reuse of the shape to always be assumed to be a recursive type.
     private Object withCopiedVisitors(Supplier<Object> supplier) {
         // Account for recursive shapes at the current
         Set<MemberShape> visitedCopy = new HashSet<>(visited);
@@ -253,7 +252,7 @@ private int computeLength(Shape shape) {
     }
 
     private double computeRange(Shape shape) {
-        // Create a random string that does not exceed or go under the length trait.
+        // Create a random string that does not exceed or go under the range trait.
         double i = 8;
 
         if (shape.hasTrait(RangeTrait.class)) {

smithy-waiters/src/main/java/software/amazon/smithy/waiters/PathComparator.java

@@ -20,7 +20,7 @@
 import software.amazon.smithy.model.node.ToNode;
 
 /**
- * Defines a comparison to perform in a ListPathMatcher.
+ * Defines a comparison to perform in a PathMatcher.
  */
 public enum PathComparator implements ToNode {
 
@@ -43,9 +43,9 @@ public enum PathComparator implements ToNode {
     }
 
     /**
-     * Creates a {@code ListPathComparator} from a {@link Node}.
-     * @param node Node to create the {@code ListPathComparator} from.
-     * @return Returns the created {@code ListPathComparator}.
+     * Creates a {@code PathComparator} from a {@link Node}.
+     * @param node Node to create the {@code PathComparator} from.
+     * @return Returns the created {@code PathComparator}.
      * @throws ExpectationNotMetException if the given {@code node} is invalid.
      */
     public static PathComparator fromNode(Node node) {

smithy-waiters/src/main/java/software/amazon/smithy/waiters/PathMatcher.java

@@ -80,8 +80,8 @@ public PathComparator getComparator() {
     /**
      * Creates a new instance from a {@link Node}.
      *
-     * @param node Node tom create the ListPathMatcher from.
-     * @return Returns the created ListPathMatcher.
+     * @param node Node tom create the PathMatcher from.
+     * @return Returns the created PathMatcher.
      * @throws ExpectationNotMetException if the given Node is invalid.
      */
     public static PathMatcher fromNode(Node node) {

smithy-waiters/src/main/resources/META-INF/smithy/waiters.smithy

@@ -29,9 +29,8 @@ structure Waiter {
     minDelay: WaiterDelay,
 
     /// The maximum amount of time in seconds to delay between each retry.
-    /// This value defaults to 256 if not specified (or, 4 minutes and 16
-    /// seconds). If specified, this value MUST be greater than or equal
-    /// to 1.
+    /// This value defaults to 120 if not specified (or, 2 minutes). If
+    /// specified, this value MUST be greater than or equal to 1.
     maxDelay: WaiterDelay,
 }
 
@@ -126,7 +125,7 @@ structure PathMatcher {
     comparator: PathComparator,
 }
 
-/// Defines a comparison to perform in a ListPathMatcher.
+/// Defines a comparison to perform in a PathMatcher.
 @enum([
     {
         "name": "STRING_EQUALS",