Uh oh!
There was an error while loading. Please reload this page.
chore: inline needlessly extracted single-use helpers - #14048
Merged
Conversation
thaJeztah
previously approved these changes
Aug 14, 2026
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
+104
to
+112
| func setServiceScale(project *types.Project, name string, replicas int) error { | ||
| service, err := project.GetService(name) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| service.SetScale(replicas) | ||
| project.Services[name] = service | ||
| return nil | ||
| } |
Member
There was a problem hiding this comment.
Looks like this is used in two places, but possibly even worth considering inlining it in both places;
ContributorAuthor
There was a problem hiding this comment.
I would rather keep this one: it has two real callers (applyScaleOpts in create.go and runScale), and the value-semantics dance (GetService → SetScale → write back into project.Services[name]) is easy to get wrong — forgetting the write-back is a silent no-op. Before this PR runScale had exactly that duplication, drifting from the helper. Real reuse is the boundary this PR tries to preserve; happy to inline both if you feel strongly about it.
Member
There was a problem hiding this comment.
Ah, right yeah, the project.Services[name] = service may be easily overlooked. Yup, that's fair, no problem!
Member
There was a problem hiding this comment.
(I was actually looking; couldn't we just iterate over project.Services, but there was also "disabled services" etc to take into account).
Uh oh!
There was an error while loading. Please reload this page.
Inline small functions that were extracted from their single call site without a real boundary to justify it — no reuse, no dedicated test, no responsibility of their own — so each caller now tells its whole story top-down: - getExecTarget, attachContainer, logContainer: one-line trampolines to getSpecifiedContainer / doAttachContainer / doLogContainer, which other call sites already use directly - removeImage: single-statement wrapper, unlike its removeVolume sibling which has actual logic - checkSelectedServices: named like a validation, actually a filter; the subtle rule (an unknown service is only an error with an explicit compose file) now reads where options.Services is rewritten - prepareLabels: mutated the map it received while looking pure at the call site; the label writes are now visible in getCreateConfigs - setDefaultTarget: mutation-by-pointer of the loop copy, now visible in the loop of injectFileReferences - buildVolumeOptions/buildTmpfsOptions/buildImageOptions: nil-guard + field copies; the buildMountOptions switch now shows side by side what each mount type propagates (buildBindOption keeps real logic and keeps buildMountOptions under the gocyclo limit) - displayDryRunBuildEvent: was longer than its only caller - hasMore: read like a predicate, was a one-line spinner restart - escapeDollarSign: wrapped a single bytes.ReplaceAll - extractEnvCLIDefined: replaced by the canonical compose-go helper types.NewMappingWithEquals().ToMapping(), as run.go already does - isPullPolicyValid: rebuilt the valid-values slice on every call; now a package-level list checked at the call site - viewFromStackList: projection now sits next to the render closure that consumes exactly its three fields No behavior change; single gocyclo threshold untouched. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
validateNavigationMenu validated nothing: it resolves the TTY / COMPOSE_MENU / --menu precedence and mutates opts.navigationMenu — rename to resolveNavigationMenu. runScale duplicated setServiceScale's GetService/SetScale/write-back dance inline; move the helper next to its natural home in scale.go and use it from both callers. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ndeloofforce-pushed
the
cleanup-needless-extractions
branch
from
August 14, 2026 17:05
0077e76 to
a5992f6CompareUh oh!
There was an error while loading. Please reload this page.
tmeijn pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Aug 19, 2026
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | minor | `v5.4.0` → `v5.5.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>docker/compose (docker/compose)</summary> ### [`v5.5.0`](https://github.com/docker/compose/releases/tag/v5.5.0) [Compare Source](docker/compose@v5.4.0...v5.5.0) #### What's Changed > ℹ️ This release overhauls image digest reconciliation to prevent unnecessary container recreation.\ > Existing containers may be recreated the first time you run `compose up` after upgrading, as image digests are re-evaluated using the new logic. > > `compose pull` now honors `pull_policy` refresh windows (`daily`, `weekly`, `every_N`). ##### ✨ Improvements - New image digest reconciliation process by [@​glours](https://github.com/glours) & [@​ndeloof](https://github.com/ndeloof) [#​14011](docker/compose#14011) [#​14041](docker/compose#14041) ##### 🐛 Fixes - Fix(bridge): skip pulling default image references for build-only ser… by [@​ericwyles](https://github.com/ericwyles) in [#​14010](docker/compose#14010) - Fix(watch): stop pruning every dangling image of the project by [@​glours](https://github.com/glours) in [#​14012](docker/compose#14012) - Fix(config): resolve service environment when computing --hash by [@​glours](https://github.com/glours) in [#​14002](docker/compose#14002) - Fix(watch): skip unreadable directories instead of failing the watch by [@​Endika](https://github.com/Endika) in [#​13992](docker/compose#13992) - Fix: ignore one-off container events in up monitor by [@​brano-osif](https://github.com/brano-osif) in [#​14038](docker/compose#14038) - Fix(bridge): validate arguments of bridge subcommands by [@​glours](https://github.com/glours) in [#​14003](docker/compose#14003) - Fix(images): tolerate containers whose image record is gone by [@​ndeloof](https://github.com/ndeloof) in [#​14028](docker/compose#14028) ##### 🔧 Internal - Test: Set stop\_signal to SIGTERM in nginx-based services by [@​ricardobranco777](https://github.com/ricardobranco777) in [#​13881](docker/compose#13881) - Chore: inline needlessly extracted single-use helpers by [@​ndeloof](https://github.com/ndeloof) in [#​14048](docker/compose#14048) - Add ENGINE column driven by label by [@​nicksieger](https://github.com/nicksieger) in [#​13959](docker/compose#13959) ##### ⚙️ Dependencies - Build(deps): bump github.com/moby/moby/client from 0.5.0 to 0.5.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13999](docker/compose#13999) - Build(deps): bump github/codeql-action/upload-sarif from 4.37.3 to 4.37.4 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14009](docker/compose#14009) - Build(deps): bump github/codeql-action/upload-sarif from 4.37.4 to 4.37.5 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14019](docker/compose#14019) - Build(deps): bump github.com/moby/buildkit from 0.32.1 to 0.32.2 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14033](docker/compose#14033) - Build(deps): bump github.com/docker/buildx from 0.36.0 to 0.36.1 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14034](docker/compose#14034) - Build(deps): bump docker/github-builder/.github/workflows/bake.yml from 1.15.0 to 1.16.0 in the docker-actions group by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14035](docker/compose#14035) - Build(deps): bump github.com/moby/go-archive from 0.3.2 to 0.3.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14043](docker/compose#14043) - Build(deps): bump github/codeql-action/upload-sarif from 4.37.5 to 4.37.6 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14022](docker/compose#14022) - Build(deps): bump github.com/docker/cli from 29.6.2+incompatible to 29.7.2+incompatible by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14042](docker/compose#14042) - Build(deps): bump google.golang.org/grpc from 1.82.1 to 1.83.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​14008](docker/compose#14008) - Bump golang to version 1.26.6 by [@​glours](https://github.com/glours) in [#​14045](docker/compose#14045) #### New Contributors - [@​ericwyles](https://github.com/ericwyles) made their first contribution in [#​14010](docker/compose#14010) - [@​Endika](https://github.com/Endika) made their first contribution in [#​13992](docker/compose#13992) - [@​brano-osif](https://github.com/brano-osif) made their first contribution in [#​14038](docker/compose#14038) **Full Changelog**: <docker/compose@v5.4.0...v5.5.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODguMCIsInVwZGF0ZWRJblZlciI6IjQzLjI4OC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Systematic "extract method" helps nobody when the extracted function has a single caller, no test, and no responsibility of its own: the reader jumps around instead of reading the flow top-down. This PR inlines the small helpers where the extraction had no real boundary — actual reuse, testability or a responsibility seam — and keeps everything that has one.
Inlined (single caller, no dedicated test, body was a plain slice of its caller's flow): one-line trampolines that other call sites already bypass (
getExecTarget,attachContainer,logContainer,removeImage); helpers whose name hid a mutation or promised something else (prepareLabelsmutated the map it received,checkSelectedServicesfiltered rather than checked,hasMoreread like a predicate but restarted a spinner); and chunks whose name added nothing over the code itself (displayDryRunBuildEvent— longer than its caller,escapeDollarSign,buildVolume/Tmpfs/ImageOptions,viewFromStackList,extractEnvCLIDefined→ canonicaltypes.NewMappingWithEquals().ToMapping(),isPullPolicyValid→ package-level list +slices.Contains).Renamed/shared instead of inlined:
validateNavigationMenu→resolveNavigationMenu(it resolves and mutates, validates nothing);setServiceScalemoved to scale.go and now used byrunScale, which duplicated its four lines inline.Deliberately untouched: the
xxxCommand/runXxxcobra idiom (uniform across the package), symmetric families (toBake*,ensure*Down, …), callbacks/iterators, conversions with a real seam, everything protected by the gocyclo 16 threshold (prepareRunsits exactly at 16), andprintEvent's color switch — it differs fromcolorFnonapi.Working(SuccessColor vs nocolor), so unifying them would change the display.No behavior change;
golangci-lint(incl. gocyclo) and unit tests pass.