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

testing: add method to notify containers through a channel #113

Closed
wants to merge 1 commit into from

Conversation

fsouza
Copy link
Owner

@fsouza fsouza commented Jun 29, 2014

@gravis, can you take a look? :)

Related to issue #105.

@gravis
Copy link

gravis commented Jun 29, 2014

LGTM.

I will test in real situation and let you know, but it looks very nice.
Thanks!

@fsouza
Copy link
Owner Author

fsouza commented Jun 29, 2014

Great! Can you test it using this branch? Then we can evolve the solution based on your real feedback, before merging. :)

@gravis
Copy link

gravis commented Jul 2, 2014

Hi,

Looks good to me, but I still can't replace my fake docker server with this version. I still need to be able to orchestrate the containers behavior with something like you proposed:

Server.CustomHandler(path string, handler http.Handler)

I suggest to merge this PR, and start a new issue for this purpose, what do you think?

@fsouza
Copy link
Owner Author

fsouza commented Jul 2, 2014

Seems fair. Are you going to send the pull request for the handler customisation or should I do it? :)

@fsouza fsouza closed this Jul 2, 2014
@fsouza fsouza deleted the issue-105 branch July 2, 2014 20:02
@fsouza
Copy link
Owner Author

fsouza commented Jul 2, 2014

It's merged now!

@gravis
Copy link

gravis commented Jul 2, 2014

Thanks!
If you have time, it would be nice otherwise I'll give a try next week.

@fsouza
Copy link
Owner Author

fsouza commented Jul 2, 2014

No problem! I will probably put something together before the end of this week.

Thanks for your feedback.

fsouza added a commit that referenced this pull request Jul 4, 2014
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