Skip to content

user: MkdirAllAndChown, MkdirAndChown: DRY - #244

Open
thaJeztah wants to merge 1 commit into
moby:mainfrom
thaJeztah:user_DRY
Open

user: MkdirAllAndChown, MkdirAndChown: DRY#244
thaJeztah wants to merge 1 commit into
moby:mainfrom
thaJeztah:user_DRY

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

No description provided.

Copilot AI lite review requested due to automatic review settings August 5, 2026 07:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors directory-creation helpers in user/ to reduce duplicated option parsing by forwarding MkdirOpt variadic options directly into the platform-specific mkdirAs implementation.

Changes:

  • Simplifies MkdirAllAndChown and MkdirAndChown to forward opts ...MkdirOpt directly to mkdirAs.
  • Updates mkdirAs (unix/windows) signatures to accept opts ...MkdirOpt and parses options inside the unix implementation.
  • Switches unix logic from onlyNew parameter usage to options.onlyNew.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
user/idtools.go Removes duplicated option parsing in public wrappers by forwarding opts to mkdirAs.
user/idtools_windows.go Updates mkdirAs signature to accept variadic MkdirOpt on Windows.
user/idtools_unix.go Updates mkdirAs signature and moves MkdirOpt parsing into the unix implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread user/idtools_unix.go
Comment thread user/idtools_windows.go
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copilot AI review requested due to automatic review settings August 5, 2026 08:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

user/idtools_unix.go:29

  • The comment below still refers to an onlyNew boolean parameter, but mkdirAs now takes variadic MkdirOpts and derives onlyNew from options.onlyNew. Updating the comment will keep it accurate and avoid confusion about how the behavior is controlled.
		if options.onlyNew {

user/idtools_windows.go:9

  • Typo in the preceding comment: "could be support here" should be "could be supported here".
func mkdirAs(path string, _ os.FileMode, _, _ int, _ bool, _ ...MkdirOpt) error {

@thaJeztah
thaJeztah marked this pull request as ready for review August 5, 2026 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants