Skip to content

Conversation

@gkallingal
Copy link

Review on Reviewable

@stefansiegl stefansiegl self-assigned this Oct 22, 2015
@stefansiegl
Copy link
Owner

Hi @gkallingal,

Thank you for this PR. Here is my thoughts:

I like the idea of cleaning up the docker file with the environment variable option. Makes it way cleaner and easier to add another version later on.

You are overwriting the files to create Introscope9.6.0.0 related Introscope docker containers. I would rather like us to have the option to use this project to build docker images for all desired Introscope versions. In the project I am working at, we are still using 9.6 (for some reasons). Integrating your changes would disallow us to create images for 9.6.

My proposal would be to have additional folders and files to be able to deal with the 9.7 version, that is for example have a /enterprise-manager/9.7/... and /database/9.7/... This would allow the us to create images for 9.6 and 9.7 (and most likely 10.x which I will provide as soon as our project migrates).

Another point: I did not have a look at 9.7 (and currently do not have the binaries here). I had to some challenges in creating the docker image for the postgres database and fill it with the necessary contents. You did not change this part. Is this on purpose (as there are no changed files within the database in 9.7)? I suspected that there is new scripts for 9.7 (have a look at https://github.com/stefansiegl/docker-introscope/tree/master/database/9.6.0.0/scripts).

Thanks,
Stefan

ggrossbe referenced this pull request in CA-APM/docker-introscope Jun 10, 2016
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