Skip to content

feat: add --separate-scratch flag#683

Merged
patrickhoefler merged 3 commits intomainfrom
feat/separate-scratch
Jul 12, 2025
Merged

feat: add --separate-scratch flag#683
patrickhoefler merged 3 commits intomainfrom
feat/separate-scratch

Conversation

@patrickhoefler
Copy link
Copy Markdown
Owner

@patrickhoefler patrickhoefler commented Jul 12, 2025

This pull request introduces several changes to improve the functionality, testing, and maintainability of the dockerfile2dot tool. The most significant updates include the addition of a --separate-scratch flag to handle scratch images uniquely, refactoring to use ID instead of Name for external image identification, and enhancements to the testing guidelines and code quality documentation.

New Feature: --separate-scratch Flag

  • Added a --separate-scratch flag to create separate nodes for each scratch image in the graph instead of collapsing them. This includes updates to the CLI (internal/cmd/root.go) and corresponding tests (internal/cmd/root_test.go). [1] [2] [3] [4] [5] [6]

Refactoring: External Image Identification

  • Refactored to use ID instead of Name for identifying external images, ensuring unique identification and improving maintainability. Changes span multiple files including internal/dockerfile2dot/build.go, internal/dockerfile2dot/convert.go, and their respective tests. [1] [2] [3] [4] [5] [6] [7]

Documentation and Testing Guidelines

  • Added a new .github/copilot-instructions.md file outlining testing guidelines and code quality standards, emphasizing clean testing, proper assertions, and consistent naming conventions.

Build Process Improvements

  • Introduced a check target in the Makefile that runs both linting and tests, streamlining the build process.

Minor Enhancements and Fixes

  • Updated addExternalImages to handle the separateScratch flag, ensuring unique IDs for scratch instances and avoiding duplicate entries. [1] [2]
  • Adjusted test cases in internal/dockerfile2dot/convert_test.go to align with the new ID-based external image identification and the --separate-scratch functionality. [1] [2] [3]

Copilot AI review requested due to automatic review settings July 12, 2025 19:26

This comment was marked as outdated.

@patrickhoefler patrickhoefler requested a review from Copilot July 12, 2025 19:49
Copy link
Copy Markdown

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 PR adds a new --separate-scratch flag to control whether scratch images are collapsed into one node or kept separate, refactors external-image handling to use unique IDs, and updates tests, documentation, and the build system accordingly.

  • Introduce --separate-scratch flag in CLI and propagate through parsing and graph generation
  • Refactor ExternalImage and WaitFor to include an ID field and update all references
  • Enhance tests and documentation; add a check Makefile target

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/dockerfile2dot/structs.go Added ID fields and improved comments
internal/dockerfile2dot/load.go Updated function signature to accept separateScratch
internal/dockerfile2dot/load_test.go Simplified test to only check error, removed output assertions
internal/dockerfile2dot/convert.go Propagate separateScratch flag and update external images logic
internal/dockerfile2dot/convert_test.go Added separateScratch cases and updated expected IDs
internal/dockerfile2dot/build.go Use waitFor.ID instead of name when creating edges
internal/dockerfile2dot/build_test.go New test for separate-scratch graph labels
internal/cmd/root.go Register --separate-scratch flag
internal/cmd/root_test.go CLI integration test for the new flag
README.md Documented the new flag
Makefile Added check target
.github/copilot-instructions.md Added testing and quality guidelines
Comments suppressed due to low confidence (2)

internal/dockerfile2dot/load_test.go:54

  • The test no longer verifies the returned SimplifiedDockerfile value, reducing coverage of parsing logic. Consider reintroducing assertions (e.g., using cmp.Diff or explicit field checks) to validate the parsed output.
			_, err := LoadAndParseDockerfile(

README.md:157

  • The indentation of the --separate-scratch entry doesn’t align with the other flags; adjust leading spaces so it lines up consistently in the flags list.
      --separate-scratch        create separate nodes for each scratch image instead of collapsing them

@patrickhoefler patrickhoefler self-assigned this Jul 12, 2025
@patrickhoefler patrickhoefler merged commit cc1295f into main Jul 12, 2025
8 checks passed
@patrickhoefler patrickhoefler deleted the feat/separate-scratch branch July 12, 2025 19:54
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