refactor: extract repeated string literals flagged by goconst#599
Conversation
Replace high-volume duplicated string literals with named constants
where the strings represent a shared concept (closed enum, wire
contract, default value), and suppress the rest with rationale where
extraction would add clutter without benefit. Resolves the goconst
findings reported by CI (golangci-lint v2.12.1).
Closed-enum extractions (exported where reusable across packages):
- core/schedule_keywords.go (new): cron schedule keywords (DailySchedule,
HourlySchedule, etc.); core/scheduler.go now references them in
IsTriggeredSchedule. cli/config_validate.go and cli/init.go reference
them too. config/schedule_keywords.go (new) duplicates the
@triggered/@manual/@none subset locally because the config package
sits below core in the import graph.
- cli/doctor.go: categoryConfiguration / categoryDocker /
categoryDockerImages / categoryJobSchedules and checkNameConnectivity.
- cli/logging.go: LogLevel{Trace,Debug,Info,...} for the canonical and
legacy/logrus level names.
- cli/deprecations.go: DeprecatedSlackWebhook, DeprecatedPollInterval,
DeprecatedNoPoll, plus normalized-key forms; referenced from
cli/docker-labels.go.
- cli/daemon.go: defaultWebAddr, defaultPprofAddr — pair with the
default: struct tags so the comparisons no longer drift.
- core/annotations.go: AnnotationKey* for the wire-level annotation
keys attached to every execution.
- core/adapters/docker/auth.go: dockerHubRegistry,
dockerHubRegistryLegacy, dockerHubAuthEndpoint.
- core/domain/image.go: DefaultImageTag for the implicit "latest" tag.
- middlewares/execution_status.go (new): statusSuccessful / Failed /
Skipped, notificationVarJob / Execution, contentTypeJSON; referenced
from mail, save, webhook, preset.
Suppressions with per-site rationale:
- cli/config_webhook.go "secret": pinned by gcfg:"secret" struct tag.
- cli/docker-labels.go "true": Docker labels are stringly-typed.
- core/docker_sdk_provider.go / core/resilience.go "name": coincidental
collision with other unrelated "name" map keys.
- config/validator.go switch cases (schedule/email-to/log-level): INI
key names pinned by struct tags elsewhere; comment above the switch
explains.
No behavior changes; all string values preserved.
Signed-off-by: Sebastian Mendel <[email protected]>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
✅ Mutation Testing ResultsMutation Score: 100.00% (threshold: 60%)
What is mutation testing?Mutation testing measures test quality by introducing small changes (mutations) to the code and checking if tests detect them. A higher score means better test effectiveness.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #599 +/- ##
==========================================
- Coverage 87.20% 87.19% -0.02%
==========================================
Files 88 88
Lines 10625 10631 +6
==========================================
+ Hits 9266 9270 +4
- Misses 1118 1119 +1
- Partials 241 242 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase to resolve goconst findings by extracting high-volume repeated string literals into named constants (where the strings represent stable shared concepts such as schedule keywords, log levels, wire keys), and adds targeted //nolint:goconst suppressions with rationale where extraction would not add value. The stated goal is no behavior change, preserving all wire/string values exactly.
Changes:
- Introduces new constant vocabularies for cron schedule keywords (
core/) and notification payload/status strings (middlewares/), then rewires call sites to use them. - Consolidates/standardizes repeated CLI/config-layer literals (log levels, doctor categories, daemon defaults, deprecated option names) into constants.
- Adds targeted
goconstsuppressions with contextual comments for “coincidental collision” literals and struct-tag-pinned strings.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| middlewares/webhook.go | Switches execution status strings and template payload map keys to shared middleware constants. |
| middlewares/save.go | Uses shared notification payload key constants for saved JSON context. |
| middlewares/preset.go | Uses shared JSON Content-Type constant for default preset headers. |
| middlewares/mail.go | Uses shared notification payload key constants and shared execution status strings. |
| middlewares/execution_status.go | Adds centralized constants for notification status labels, payload keys, and default JSON content type. |
| core/scheduler.go | Updates triggered-schedule checks to use shared schedule keyword constants. |
| core/schedule_keywords.go | Adds exported constants for all recognized @... schedule keywords (time-based and triggered-only). |
| core/resilience.go | Adds a localized goconst suppression for a metrics map key literal. |
| core/domain/image.go | Extracts Docker’s implicit "latest" tag into DefaultImageTag. |
| core/docker_sdk_provider.go | Adds a localized goconst suppression for Docker SDK filter key "name". |
| core/annotations.go | Extracts default execution annotation keys into exported constants and uses them when building the default map. |
| core/adapters/docker/auth.go | Extracts Docker Hub registry/auth endpoint strings into named constants and uses them in normalization/fallback logic. |
| config/validator.go | Uses config-local schedule keyword constants and config-local log level constants; adds targeted goconst suppressions with rationale for INI-key switches. |
| config/schedule_keywords.go | Adds config-local constants for triggered schedule keywords and log level strings to avoid import cycles. |
| config/sanitizer.go | Uses config-local triggered schedule keyword constants during cron expression validation. |
| cli/logging.go | Extracts log level string literals into exported constants and uses them in ApplyLogLevel. |
| cli/init.go | Uses shared log level constants in prompts, adds a default admin username constant, and uses core schedule keyword constants for defaults/validation. |
| cli/doctor.go | Extracts doctor category/check-name strings into constants and uses them across report building and icon mapping. |
| cli/docker-labels.go | Uses deprecated-option constant for the global label allowlist; adds targeted goconst suppression for label value "true". |
| cli/docker_config_handler.go | Extracts Docker event filter type "container" into a constant and uses it in event subscription filters. |
| cli/deprecations.go | Extracts deprecated option names into exported constants and normalizes internal key forms into constants. |
| cli/daemon.go | Extracts default web/pprof listen addresses into constants and uses them in “still default?” comparisons. |
| cli/config_webhook.go | Adds targeted goconst suppression for a struct-tag-pinned "secret" case label. |
| cli/config_validate.go | Uses core schedule keyword constants in cron validation’s special-values allowlist. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase to replace hardcoded string literals with named constants across multiple packages, including CLI, core, and middlewares. New files were introduced to centralize these constants, such as core/schedule_keywords.go and middlewares/execution_status.go, while nolint:goconst directives were added to handle cases where literals are necessary for Go syntax or to avoid false positives. I have no feedback to provide as there were no review comments to assess.
The error message hardcoded only debug/info/warn/error but ApplyLogLevel also accepts trace/notice/warning/fatal/panic/critical as logrus-era aliases. Build the message from the validLogLevels slice so it stays in sync with the switch. Addresses Copilot review on cli/logging.go:51. Signed-off-by: Sebastian Mendel <[email protected]>
There was a problem hiding this comment.
Automated approval for maintainer PR
All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.
|
Summary
Resolves the
goconstfindings surfaced by golangci-lint v2.12.1 (CI's pinned version). Replaces high-volume duplicated string literals with named constants where the strings represent a shared concept — closed enum, wire contract, default value — and suppresses the rest with per-site rationale where extraction would add clutter without benefit.No behavior changes. All string values are preserved exactly.
What changed
Closed-enum extractions
core/schedule_keywords.go(new)@*schedules (Daily, Hourly, Weekly, Yearly, Annually, Monthly, Midnight, Manual, Triggered, None). Exported.core/scheduler.go'sIsTriggeredSchedulenow references them.cli/config_validate.go,cli/init.goreference the same vocabulary.config/schedule_keywords.go(new)configsits belowcorein the import graph so the@triggered/@manual/@nonetriplet is duplicated locally with a comment explaining the layering. Also includeslogLevel*mirrors used byconfig/validator.go'svalidLevelsslice.cli/doctor.gocategoryConfiguration,categoryDocker,categoryDockerImages,categoryJobSchedules, pluscheckNameConnectivityfor the recurring sub-name. Replaces 28Category:literals plus the emoji map.cli/logging.goLogLevel{Trace,Debug,Info,Notice,Warn,Warning,Error,Fatal,Panic,Critical}. Used in the switch +cli/init.go's prompt Items list.cli/deprecations.goDeprecatedSlackWebhook,DeprecatedPollInterval,DeprecatedNoPollplus unexported normalized-key forms. Referenced fromcli/docker-labels.go's allow-list.cli/daemon.godefaultWebAddr(":8081") anddefaultPprofAddr("127.0.0.1:8080") — used by the four "is the user still on the default?" comparisons.core/annotations.goAnnotationKey{JobName,JobType,ExecutionTime,SchedulerHost,Version}— wire-level identifiers attached to every execution.core/adapters/docker/auth.godockerHubRegistry,dockerHubRegistryLegacy,dockerHubAuthEndpoint.core/domain/image.goDefaultImageTag = "latest".middlewares/execution_status.go(new)statusSuccessful/Failed/Skipped,notificationVarJob/Execution,contentTypeJSON. Referenced frommail.go,save.go,webhook.go,preset.go.Targeted suppressions (with per-site rationale)
cli/config_webhook.go"secret"gcfg:"secret"struct tag value — Go syntax forces tag literals.cli/docker-labels.go"true""true"is the contract, not a missing constant.core/docker_sdk_provider.go"name""name"literals elsewhere.core/resilience.go"name"config/validator.goswitch cases (schedule,email-to,log-level)gcfg:"…"/mapstructure:"…"struct tags onConfig; extracting consts only relocates the duplication. Comment above the switch explains.Why suppressions, not more extractions
goconstmatches text repetition; the question is whether each match expresses one concept or coincidental text. Six unrelatedcasestatements that happen to contain the word"name"aren't sharing a concept — hoistingconst cName = "name"doesn't prevent any bug, it just adds clutter. Where Go's struct-tag syntax forces a literal anyway (it can't reference constants), an extracted const lives next to the literal rather than replacing it.Test plan
go build ./...cleango test -count=1 -short ./...— all packages passgolangci-lint run --enable-only=goconst ./...— 0 issues (was 51)golangci-lint run ./...— 0 issues