Skip to content

[build-script] Split execute_one_impl_action. #23917

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

Conversation

drodriguez
Copy link
Contributor

execute_one_impl_action is checking for the validity of its arguments, when it
can be enforced by the structure by using smaller focused methods that trickle
down into a common function.

This structure will eventually allow different builders to participate in a
script invocation. Each builder will be told to build/test/install, and they
will not have to deal with anything else. The content of these smaller methods
will end up as part of the builder that uses build-script-impl.

This was part of #23257. I’m trying to split the PR in smaller ones to make the reviews easier. Other PRs in this group are #23303, #23803, #23810, #23822, #23865, and #23915.

execute_one_impl_action is checking for the validity of its arguments, when it
can be enforced by the structure by using smaller focused methods that trickle
down into a common function.

This structure will eventually allow different builders to participate in a
script invocation. Each builder will be told to build/test/install, and they
will not have to deal with anything else. The content of these smaller methods
will end up as part of the builder that uses build-script-impl.
@compnerd
Copy link
Member

Seems like this is a NFC change, but the code is way more readable.

@compnerd
Copy link
Member

@swift-ci please test

Copy link
Contributor

@Rostepher Rostepher left a comment

Choose a reason for hiding this comment

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

LGTM!

@compnerd compnerd merged commit 4de9fa6 into swiftlang:master Apr 10, 2019
@drodriguez drodriguez deleted the build-script-split-execute-one-impl-action branch April 10, 2019 18:33
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.

3 participants