Skip to content
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

refactor(reverse): dockerfilefromhistory default processor #472

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ess
Copy link

@ess ess commented Feb 6, 2023

Per the desire expressed in the TODO in the default raw instruction processing case in DockerfileFromHistory, this adds tests for that processor and refactors it.

The end result increases cumulative cyclomatic complexity a few points, but generally drops the complexity of the individual pieces through conditional flattening and code deduplication, provides a clear path towards extension, and generally makes the default processor a good deal easier to reason about.

In order to test and refactor the default case for the raw
instruction processor, this change identify the values affected
(`inst`and `isExecForm`), and extracts the case body out to
a new function.

Signed-off-by: Dennis Walters <[email protected]>
Added unit tests for the function that was previously extracted
from `DockerfileFromHistory` to ensure refactored behavior.

Signed-off-by: Dennis Walters <[email protected]>
* Extracts `processAsUncaughtInst` for handling standard
  instructions that were given in a non-standard form (as
  may happen with buildkit, etc)
* Extracts `processAsArgsFormat` for handling RUN instructions
  that were provided in the args format
* Extracts `processAsExecRun` for handling commands that were
  provided in a non-standard way
* Extracts `detectRawShellForm` for detecting commands that
  are shaped like shell form RUN instructions
* Extracts `execify` for converting arrays of strings to
  exec format RUN instructions (and lowering code duplication)
* Replaces the body of `processRawInst` to incorporate these
  extracted functions and generally make the problem easier
  to reason about and extend

Signed-off-by: Dennis Walters <[email protected]>
Signed-off-by: Dennis Walters <[email protected]>
Signed-off-by: Dennis Walters <[email protected]>
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.

1 participant