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

docker-compose build support #12

Merged
merged 3 commits into from
Jun 29, 2018
Merged

docker-compose build support #12

merged 3 commits into from
Jun 29, 2018

Conversation

zaucy
Copy link
Contributor

@zaucy zaucy commented Jun 28, 2018

I used argument name service because of issue #9. I can change it to container to match the other methods if you would like.

If TypeScript was used (#10) the signature for the build function could be specified like so:

build(service: string, options: IBuildOptions): Promise<string>
build(services: string[], options: IBuildOptions): Promise<string>
build(options: IBuildOptions): Promise<string>

build(arg0: string|string[]|IBuildOptions, arg1?: options: IBuildOptions) {
  // ...
}

I can understand not wanting to use the same name for each of the above signatures. I can rename them to buildOne(), buildMany(), buildAll() if you'd like.

@zaucy zaucy changed the title docker-compose build support #11 docker-compose build support Jun 28, 2018
@AlexZeitler
Copy link
Contributor

Thanks for the PR. I would prefer buildOne(), buildMany(), buildAll().

@zaucy
Copy link
Contributor Author

zaucy commented Jun 28, 2018

Alright sounds good. I'll change them.

I'm curious if there is intended support for the other commands with the singular and many options such as up. Would there be an upAll() upMany() upOne()? I can add the different types to all of them if that sounds good to you.

@AlexZeitler
Copy link
Contributor

AlexZeitler commented Jun 28, 2018

Great, I created #18 for upAll(), upMany() and upOne().

@zaucy
Copy link
Contributor Author

zaucy commented Jun 29, 2018

I changed build() to be buildOne(), buildMany() and buildAll().

I also broke exec() and run() on my last PR with the spawn change. I included the fix here. The execCompose() was also not resolving it's promise, because spawn doesn't have a callback for an argument. Instead I listen to the stdout/stderr streams and construct a response object that is the same as the old version for backwards compatibility. Sorry about that!

@AlexZeitler AlexZeitler merged commit b29f6c4 into PDMLab:master Jun 29, 2018
@AlexZeitler
Copy link
Contributor

Thanks, good job! 👍

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.

None yet

2 participants