Skip to content

Conversation

euanh
Copy link
Collaborator

@euanh euanh commented Apr 28, 2025

Motivation

Most of containertool's work is handled in its run() method.

  • It starts by processing arguments and assigning defaults;
  • It then creates RegistryClient instance;
  • Finally it assembles the image and uploads it.

All of this work takes place inline, in a single method called by SwiftArgumentParser. This complicates maintenance and makes it difficult to test the image assembly process without writing full end-to-end tests.

Modifications

  • Extract the image building process into a standalone function.
  • run() is only responsible for processing arguments and creating the clients needed by the build function.

Result

The image build process will be easier to test and maintain.
It will be easier to add more complex argument handling, such as additional environment variables or a configuration file.

Test Plan

Existing tests continue to pass.
Future pull requests can test the image build process in integration or unit tests, instead of requiring full end-to-end tests.

@euanh euanh added the semver/minor Adds new public API. label Apr 28, 2025
@euanh euanh force-pushed the refactor/containertool-option-processing branch from 92e2413 to 436fdbc Compare April 28, 2025 10:41
@euanh euanh merged commit 6ed6c6c into apple:main Apr 28, 2025
23 checks passed
@euanh euanh deleted the refactor/containertool-option-processing branch April 28, 2025 10:59
euanh added a commit to euanh/swift-container-plugin that referenced this pull request May 1, 2025
…lishing (apple#116)

Motivation
----------

Most of `containertool`'s work is handled in its `run()` method.
* It starts by processing arguments and assigning defaults;
* It then creates `RegistryClient` instance;
* Finally it assembles the image and uploads it.

All of this work takes place inline, in a single method called by
`SwiftArgumentParser`. This complicates maintenance and makes it
difficult to test the image assembly process without writing full
end-to-end tests.

Modifications
-------------

* Extract the image building process into a standalone function.
* `run()` is only responsible for processing arguments and creating the
clients needed by the build function.

Result
------

The image build process will be easier to test and maintain.
It will be easier to add more complex argument handling, such as
additional environment variables or a configuration file.

Test Plan
---------

Existing tests continue to pass.
Future pull requests can test the image build process in integration or
unit tests, instead of requiring full end-to-end tests.
@euanh euanh mentioned this pull request May 1, 2025
euanh added a commit that referenced this pull request May 1, 2025
…lishing (#116)

Motivation
----------

Most of `containertool`'s work is handled in its `run()` method.
* It starts by processing arguments and assigning defaults;
* It then creates `RegistryClient` instance;
* Finally it assembles the image and uploads it.

All of this work takes place inline, in a single method called by
`SwiftArgumentParser`. This complicates maintenance and makes it
difficult to test the image assembly process without writing full
end-to-end tests.

Modifications
-------------

* Extract the image building process into a standalone function.
* `run()` is only responsible for processing arguments and creating the
clients needed by the build function.

Result
------

The image build process will be easier to test and maintain.
It will be easier to add more complex argument handling, such as
additional environment variables or a configuration file.

Test Plan
---------

Existing tests continue to pass.
Future pull requests can test the image build process in integration or
unit tests, instead of requiring full end-to-end tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant