Playing with comments

Just some copyediting.

Author: featherbread

internal/awsconfig/awsconfig.go

@@ -1,5 +1,4 @@
-// Package awsconfig generates AWS client configurations optimized for use by
-// the randomizer.
+// Package awsconfig provides a randomizer-optimized AWS SDK configuration.
 package awsconfig
 
 import (
@@ -37,10 +36,9 @@ type Option = func(*config.LoadOptions) error
 func New(ctx context.Context) (aws.Config, error) {
 	transport := http.DefaultTransport
 
-	// This option is recommended in AWS Lambda deployments due to the
-	// significant reduction in cold start latency (see getEmbeddedCertPool).
-	// It can be enabled for standard server deployments if desired, but is far
-	// less beneficial.
+	// This option is recommended in AWS Lambda to significantly reduce cold
+	// start latency (see [getEmbeddedCertTransport]). It can be enabled for
+	// standard server deployments if desired, but is far less beneficial.
 	if os.Getenv("AWS_CLIENT_EMBEDDED_TLS_ROOTS") == "1" {
 		transport = getEmbeddedCertTransport()
 	}
@@ -60,10 +58,9 @@ func New(ctx context.Context) (aws.Config, error) {
 		return aws.Config{}, fmt.Errorf("loading AWS config: %w", err)
 	}
 
-	// WARNING: X-Ray tracing will panic if the context passed to AWS operations
-	// is not already associated with an open X-Ray segment. That means that as of
-	// this writing this option is only safe to use on AWS Lambda. Standard server
-	// deployments should avoid setting it.
+	// NOTE: X-Ray tracing panics if the context for an AWS call is not already
+	// associated with an open X-Ray segment. As of writing, this option is only
+	// safe to use on AWS Lambda. Standard server deployments should avoid it.
 	if useXRay := os.Getenv("AWS_CLIENT_XRAY_TRACING"); useXRay == "1" {
 		xrayawsv2.AWSV2Instrumentor(&cfg.APIOptions)
 	}
@@ -75,12 +72,11 @@ func New(ctx context.Context) (aws.Config, error) {
 // CAs operated by Amazon Trust Services, which all AWS service endpoints chain
 // from.
 //
-// When the randomizer runs on AWS Lambda in the recommended configuration, this
-// limited set of roots is so much cheaper to parse than a typical set of system
-// roots that it cuts cold start invocation time roughly in half (by around
-// 500ms). This is a large enough difference for a human to notice, and accounts
-// for about 15% of the 3 second response time limit that Slack imposes on slash
-// commands.
+// When the randomizer runs on AWS Lambda with recommended resource settings,
+// this limited set of roots is substantially cheaper to parse than a typical
+// root store, which removes ~500ms of cold-start response latency. That's
+// large enough for a human to notice, and accounts for ~15% of the 3-second
+// response time limit Slack imposes on slash commands.
 var getEmbeddedCertTransport = sync.OnceValue(func() *http.Transport {
 	transport := http.DefaultTransport.(*http.Transport).Clone()
 	transport.TLSClientConfig = &tls.Config{RootCAs: loadEmbeddedCertPool()}

internal/randomizer/app.go

@@ -5,8 +5,7 @@ import (
 	"math/rand/v2"
 )
 
-// Store represents an object that provides persistence for "groups" of
-// options.
+// Store enables persistence for named groups of options.
 type Store interface {
 	// List returns the names of all available groups. If no groups have been
 	// saved, it returns an empty list with a nil error.
@@ -20,20 +19,18 @@ type Store interface {
 	// group with that name.
 	Put(ctx context.Context, group string, options []string) error
 
-	// Delete ensures that the named group no longer exists, returning true or
-	// false to indicate whether the group existed prior to this deletion
-	// attempt.
-	Delete(ctx context.Context, group string) (bool, error)
+	// Delete ensures that the named group no longer exists, and indicates
+	// whether the group existed prior to this deletion attempt.
+	Delete(ctx context.Context, group string) (existed bool, err error)
 }
 
-// App represents a randomizer app that can accept commands.
+// App represents a randomizer instance that can accept commands.
 type App struct {
 	name    string
 	store   Store
 	shuffle func([]string) // Overridden in tests for predictable behavior
 }
 
-// NewApp returns an App.
 func NewApp(name string, store Store) App {
 	return App{
 		name:    name,
@@ -48,11 +45,10 @@ func shuffle(options []string) {
 	})
 }
 
-// Main is the entrypoint to the randomizer tool.
+// Main is the entrypoint to the randomizer.
 //
-// Note that all errors returned from this function will be of this package's
-// Error type. This provides the HelpText method for user-friendly output
-// formatting.
+// All errors returned from Main are of type [Error], and support
+// [Error.HelpText] for user-friendly formatting.
 func (a App) Main(ctx context.Context, args []string) (Result, error) {
 	request, err := a.newRequest(ctx, args)
 	if err != nil {

internal/randomizer/app_test.go

@@ -260,12 +260,10 @@ func isResult(expectedType ResultType, contains ...string) validator {
 		message := res.Message()
 		for _, c := range contains {
 			i := strings.Index(message, c)
-
 			if i < 0 {
 				t.Errorf("result missing %q in expected position\n%v", c, res.Message())
 				continue
 			}
-
 			message = message[i+len(c):]
 		}
 	}
@@ -276,14 +274,9 @@ func isError(contains string) validator {
 		if err == nil {
 			t.Fatalf("unexpected result %v", res)
 		}
-
-		if _, ok := err.(Error); !ok {
-			t.Fatalf("unexpected error type %T", err)
-		}
-
-		rerr := err.(Error)
-
-		if !strings.Contains(rerr.HelpText(), contains) {
+		if rerr, ok := err.(Error); !ok {
+			t.Errorf("unexpected error type %T", err)
+		} else if !strings.Contains(rerr.HelpText(), contains) {
 			t.Errorf("error help text missing substring %q", contains)
 		}
 	}

internal/randomizer/request.go

@@ -32,20 +32,19 @@ func (a App) newRequest(ctx context.Context, args []string) (req request, err er
 }
 
 func parseArgs(args []string) (op operation, operand string, opargs []string, err error) {
-	// We accept the standard flag syntax for help, but strongly expect that users
-	// won't know that syntax in advance. Logic elsewhere in the randomizer
-	// prevents the use of "help" as a group name to avoid conflicts with this
-	// special case.
+	// We accept the standard flag syntax for help, but expect that users won't
+	// know that syntax in advance. Logic elsewhere in the randomizer blocks
+	// using "help" as a group name to avoid conflicts with this special case.
 	if len(args) == 0 || args[0] == "/help" || len(args) == 1 && args[0] == "help" {
 		return showHelp, "", args, nil
 	}
 
 	switch args[0] {
-	// Arguments without an explicitly known flag always trigger a randomization,
-	// even if the first argument starts with a slash, simply because it's less
-	// work to implement and unlikely to cause big problems in practice. Logic
-	// elsewhere in the randomizer prevents the use of flag-like group names, so
-	// that new flags can't make existing groups inaccessible.
+	// Arguments without an explicitly known flag trigger randomization, even if
+	// the first argument starts with a slash, because it's easier to implement
+	// and unlikely to cause problems in practice. Logic elsewhere in the
+	// randomizer blocks using flag-like group names, so new flags can't make
+	// existing groups inaccessible.
 	default:
 		return makeSelection, "", args, nil
 

internal/randomizer/rndtest/rndtest.go

@@ -9,7 +9,7 @@ import (
 )
 
 // Store implements randomizer.Store by mapping group names to sorted lists of
-// strings. A nil Store will return errors for every operation.
+// strings. A nil Store returns errors for every operation.
 type Store map[string][]string
 
 // Clone returns a deep copy of the original store.

internal/slack/slack.go

@@ -13,12 +13,13 @@ import (
 	"github.com/featherbread/randomizer/internal/randomizer"
 )
 
-// App provides HTTP handling logic that allows the randomizer to be integrated
-// as a slash command in a Slack workspace.
+// App serves the randomizer through the Slack slash command API.
 //
-// Note that App currently only supports static verification tokens to check
-// that a request legitimately originated from Slack, and does not support the
-// newer signed secrets functionality.
+// App supports legacy static verification tokens to confirm the legitimacy of
+// requests from Slack; it does not support the newer signed secrets strategy.
+//
+// App supports only HTTP POST requests; it does not support the GET requests
+// allowed by the deprecated legacy slash command integration.
 type App struct {
 	// TokenProvider provides the expected value of the slash command verification
 	// token generated by Slack. This can be obtained from the slash command
@@ -27,13 +28,11 @@ type App struct {
 	// StoreFactory provides a Store for the Slack channel in which the request
 	// was made.
 	StoreFactory func(partition string) randomizer.Store
-	// Logger, if non-nil, will be used to report information about errors that
-	// occur while handling each request.
+	// Logger, if non-nil, logs errors encountered during request handling.
 	Logger *slog.Logger
 }
 
-// ServeHTTP handles the POST request that Slack makes to invoke the randomizer
-// slash command.
+// ServeHTTP serves POST requests from Slack.
 func (a App) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodPost {
 		w.Header().Add("Allow", http.MethodPost)

internal/slack/token.go

@@ -16,19 +16,19 @@ import (
 
 const DefaultAWSParameterTTL = 2 * time.Minute
 
-// TokenProvider provides the value of the slash command verification token
-// generated by Slack.
+// TokenProvider provides the expected value of the slash command verification
+// token that Slack includes in its requests.
 type TokenProvider func(ctx context.Context) (string, error)
 
 // TokenProviderFromEnv returns a TokenProvider based on available environment
 // variables.
 //
-// If SLACK_TOKEN is set, it will return a static token provider.
+// If SLACK_TOKEN is set, it returns a static token provider.
 //
-// If SLACK_TOKEN_SSM_NAME is set, it will return an AWS SSM token provider,
+// If SLACK_TOKEN_SSM_NAME is set, it returns an AWS SSM token provider,
 // with the TTL optionally set by SLACK_TOKEN_SSM_TTL.
 //
-// Otherwise, it will return an error.
+// Otherwise, it returns an error.
 func TokenProviderFromEnv() (TokenProvider, error) {
 	if token, ok := os.LookupEnv("SLACK_TOKEN"); ok {
 		return StaticToken(token), nil

internal/store/registry/registry.go

@@ -15,10 +15,10 @@ var Registry = map[string]Entry{}
 
 // Entry represents a single store backend.
 type Entry struct {
-	// EnvironmentKeys is the list of environment variables that this store's
-	// factory checks for configuration. If any one of these keys is set in the
-	// environment (and no conflicting keys are set), this store will be selected
-	// as the backend for this randomizer instance.
+	// EnvironmentKeys lists the names of environment variables that this store's
+	// factory checks for configuration. If the environment contains any of these
+	// keys, and contains no keys for other factories, the randomizer selects this
+	// store as its backend.
 	EnvironmentKeys []string
 
 	// FactoryFromEnv creates a factory for this backend based on its environment