Skip to content

modernize code with slices and maps packages - #7268

Open
thaJeztah wants to merge 8 commits into
docker:masterfrom
thaJeztah:slice_and_dice
Open

modernize code with slices and maps packages#7268
thaJeztah wants to merge 8 commits into
docker:masterfrom
thaJeztah:slice_and_dice

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 29.8.0 milestone Sep 1, 2026
@thaJeztah
thaJeztah requested review from a team and silvin-lubecki as code owners September 1, 2026 19:40
@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Sep 1, 2026
@thaJeztah

Copy link
Copy Markdown
Member Author
ERROR: failed to solve: failed to fetch oauth token: unexpected status from POST request to https://auth.docker.io/token: 502 Bad Gateway: <html>
<head><title>502 Bad Gateway</title></head>
<body>
<center><h1>502 Bad Gateway</h1></center>
<hr><center>cloudflare</center>
</body>
</html>

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.00000% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/command/formatter/buildcache.go 0.00% 9 Missing and 1 partial ⚠️
cli/command/stack/loader.go 0.00% 2 Missing ⚠️
cli/command/service/logs.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Review incomplete

chunk 1: Drafter did not complete (schema validation error prevented drafter agent delegation; direct analysis by orchestrator was performed instead across all 20 changed production files)

Findings so far: ⚠️ INCOMPLETE


Orchestrator direct analysis summary:

This PR replaces sort, strings.HasPrefix, manual key-collection loops, and similar patterns with idiomatic slices/maps package equivalents. All replacements reviewed appear semantically equivalent:

  • slices.Sort / slices.SortFunc — correct replacements for sort.Strings / sort.Slice
  • slices.Sorted(maps.Keys(...)) — deterministically sorted, equivalent to collect-then-sort pattern
  • slices.Min(keys) — equivalent to sort.Strings(keys); keys[0]
  • slices.Concat(...) — equivalent to append(head, tail...)
  • slices.SortedFunc(maps.Values(...), ...) — correct for UlimitOpt.GetList; stable vs. unstable irrelevant since ulimit names are unique map keys
  • filepath.IsLocal path check in config.go — more robust than strings.HasPrefix; correctly rejects traversal paths
  • %s%v in toStringList — only affects nil values when allowNil=true; sole caller uses allowNil=false so no behavioral change
  • Nil-mutation fix in trust/common.go and trust/formatter.go — old code mutated the receiver's slice via sort.Strings; new code sorts a copy, which is an improvement

No bugs found in the changed production code.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Use filepath.Rel and filepath.IsLocal instead of comparing path prefixes
as strings when checking that a path stays within the config directory.

Also avoid calling Dir multiple times when constructing and validating
the path.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants