Skip to content

commands: remove debug package in commands #3240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

jsternberg
Copy link
Collaborator

The package just causes the entire flow to be more complicated as build
has to pretend it doesn't know about debug options and the debugger has
to pretend it doesn't know about the build.

This abstraction has been difficult when integrating a DAP command into
this same workflow so I don't think this abstraction has much of a
value.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request removes the legacy debug package from the commands and replaces its usage with a new debugging command implementation. Key changes include:

  • Removing the debug package code (commands/debug/root.go) entirely.
  • Updating commands/root.go to use a new debugging command.
  • Refactoring commands/build.go to accept the revised debug options type.

Reviewed Changes

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

File Description
commands/root.go Updated to remove debug package dependency and call a new debugging command
commands/debug/root.go Removed the legacy debug command implementation
commands/debug.go Introduced a new debug command using debugOptions
commands/build.go Refactored buildCmd to use debugOptions instead of debug.DebugConfig
Comments suppressed due to low confidence (1)

commands/root.go:122

  • The function 'dapCmd' is not defined in the diff and appears to be a typo. It is likely that 'debugCmd' (defined in commands/debug.go) was intended here.
cmd.AddCommand(dapCmd(dockerCli, opts))

@jsternberg jsternberg force-pushed the remove-debugcmd-package branch from 2cf3ce3 to 23788a0 Compare June 12, 2025 21:46
@jsternberg
Copy link
Collaborator Author

The copilot comment does not really seem to be correct. There's no legacy debug command or implementation. I just copied it to the same package as the other commands. I didn't want someone to get the wrong impression from it.

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Docs needs regeneration

@jsternberg jsternberg requested a review from crazy-max June 13, 2025 14:30
The package just causes the entire flow to be more complicated as build
has to pretend it doesn't know about debug options and the debugger has
to pretend it doesn't know about the build.

This abstraction has been difficult when integrating a DAP command into
this same workflow so I don't think this abstraction has much of a
value.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
@jsternberg jsternberg force-pushed the remove-debugcmd-package branch from 23788a0 to 7f5ff6b Compare June 13, 2025 14:32
@jsternberg
Copy link
Collaborator Author

The docs change was caused by removing the --progress from debug. --progress is on the build command itself and this version of progress was used by the monitor CLI that got removed. We just didn't notice that this line of code did nothing.

@tonistiigi tonistiigi merged commit 43e2f27 into docker:master Jun 13, 2025
140 checks passed
@jsternberg jsternberg deleted the remove-debugcmd-package branch June 13, 2025 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants